diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifLiveDataStoreImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifLiveDataStoreImpl.kt index d95d593778a93..5acc50ab878fb 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifLiveDataStoreImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifLiveDataStoreImpl.kt @@ -21,7 +21,6 @@ import com.android.systemui.dagger.SysUISingleton import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.util.Assert import com.android.systemui.util.ListenerSet -import com.android.systemui.util.isNotEmpty import com.android.systemui.util.traceSection import java.util.Collections.unmodifiableList import java.util.concurrent.Executor diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/DebugModeFilterProvider.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/DebugModeFilterProvider.kt index fd5bae1515505..c873e6ad36d46 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/DebugModeFilterProvider.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/DebugModeFilterProvider.kt @@ -26,7 +26,6 @@ import com.android.systemui.statusbar.commandline.CommandRegistry import com.android.systemui.statusbar.notification.collection.NotificationEntry import com.android.systemui.util.Assert import com.android.systemui.util.ListenerSet -import com.android.systemui.util.isNotEmpty import java.io.PrintWriter import javax.inject.Inject diff --git a/packages/SystemUI/src/com/android/systemui/util/IListenerSet.kt b/packages/SystemUI/src/com/android/systemui/util/IListenerSet.kt new file mode 100644 index 0000000000000..b0230b8eb0dd1 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/util/IListenerSet.kt @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2023 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.util + +/** + * A collection of listeners, observers, callbacks, etc. + * + * This container is optimized for infrequent mutation and frequent iteration, with thread safety + * and reentrant-safety guarantees as well. Specifically, to ensure that + * [ConcurrentModificationException] is never thrown, this iterator will not reflect changes made to + * the set after the iterator is constructed. + */ +interface IListenerSet : Set { + /** + * A thread-safe, reentrant-safe method to add a listener. Does nothing if the listener is + * already in the set. + */ + fun addIfAbsent(element: E): Boolean + + /** A thread-safe, reentrant-safe method to remove a listener. */ + fun remove(element: E): Boolean +} diff --git a/packages/SystemUI/src/com/android/systemui/util/ListenerSet.kt b/packages/SystemUI/src/com/android/systemui/util/ListenerSet.kt index a47e61441c4c4..f8e0b3dfe6d53 100644 --- a/packages/SystemUI/src/com/android/systemui/util/ListenerSet.kt +++ b/packages/SystemUI/src/com/android/systemui/util/ListenerSet.kt @@ -29,20 +29,12 @@ import java.util.concurrent.CopyOnWriteArrayList class ListenerSet /** Private constructor takes the internal list so that we can use auto-delegation */ private constructor(private val listeners: CopyOnWriteArrayList) : - Collection by listeners, Set { + Collection by listeners, IListenerSet { /** Create a new instance */ constructor() : this(CopyOnWriteArrayList()) - /** - * A thread-safe, reentrant-safe method to add a listener. Does nothing if the listener is - * already in the set. - */ - fun addIfAbsent(element: E): Boolean = listeners.addIfAbsent(element) + override fun addIfAbsent(element: E): Boolean = listeners.addIfAbsent(element) - /** A thread-safe, reentrant-safe method to remove a listener. */ - fun remove(element: E): Boolean = listeners.remove(element) + override fun remove(element: E): Boolean = listeners.remove(element) } - -/** Extension to match Collection which is implemented to only be (easily) accessible in kotlin */ -fun ListenerSet.isNotEmpty(): Boolean = !isEmpty() diff --git a/packages/SystemUI/src/com/android/systemui/util/NamedListenerSet.kt b/packages/SystemUI/src/com/android/systemui/util/NamedListenerSet.kt new file mode 100644 index 0000000000000..c90b57ed449f6 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/util/NamedListenerSet.kt @@ -0,0 +1,96 @@ +/* + * Copyright (C) 2023 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.util + +import java.util.concurrent.CopyOnWriteArrayList +import java.util.function.Consumer + +/** + * A collection of listeners, observers, callbacks, etc. + * + * This container is optimized for infrequent mutation and frequent iteration, with thread safety + * and reentrant-safety guarantees as well. Specifically, to ensure that + * [ConcurrentModificationException] is never thrown, this iterator will not reflect changes made to + * the set after the iterator is constructed. + * + * This class provides all the abilities of [ListenerSet], except that each listener has a name + * calculated at runtime which can be used for time-efficient tracing of listener invocations. + */ +class NamedListenerSet( + private val getName: (E) -> String = { it.javaClass.name }, +) : IListenerSet { + private val listeners = CopyOnWriteArrayList() + + override val size: Int + get() = listeners.size + + override fun isEmpty() = listeners.isEmpty() + + override fun iterator(): Iterator = iterator { + listeners.iterator().forEach { yield(it.listener) } + } + + override fun containsAll(elements: Collection) = + listeners.count { it.listener in elements } == elements.size + + override fun contains(element: E) = listeners.firstOrNull { it.listener == element } != null + + override fun addIfAbsent(element: E): Boolean = listeners.addIfAbsent(NamedListener(element)) + + override fun remove(element: E): Boolean = listeners.removeIf { it.listener == element } + + /** A wrapper for the listener with an associated name. */ + inner class NamedListener(val listener: E) { + val name: String = getName(listener) + + override fun hashCode(): Int { + return listener.hashCode() + } + + override fun equals(other: Any?): Boolean = + when { + other === null -> false + other === this -> true + other !is NamedListenerSet<*>.NamedListener -> false + listener == other.listener -> true + else -> false + } + } + + /** Iterate the listeners in the set, providing the name for each one as well. */ + inline fun forEachNamed(block: (String, E) -> Unit) = + namedIterator().forEach { element -> block(element.name, element.listener) } + + /** + * Iterate the listeners in the set, wrapping each call to the block with [traceSection] using + * the listener name. + */ + inline fun forEachTraced(block: (E) -> Unit) = forEachNamed { name, listener -> + traceSection(name) { block(listener) } + } + + /** + * Iterate the listeners in the set, wrapping each call to the block with [traceSection] using + * the listener name. + */ + fun forEachTraced(consumer: Consumer) = forEachNamed { name, listener -> + traceSection(name) { consumer.accept(listener) } + } + + /** Iterate over the [NamedListener]s currently in the set. */ + fun namedIterator(): Iterator = listeners.iterator() +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/util/ListenerSetTest.kt b/packages/SystemUI/tests/src/com/android/systemui/util/ListenerSetTest.kt index 2662da201460c..1404a4fdbdaea 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/util/ListenerSetTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/util/ListenerSetTest.kt @@ -16,43 +16,128 @@ package com.android.systemui.util -import android.test.suitebuilder.annotation.SmallTest -import androidx.test.runner.AndroidJUnit4 +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.google.common.truth.Truth.assertThat -import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @SmallTest @RunWith(AndroidJUnit4::class) -class ListenerSetTest : SysuiTestCase() { +open class ListenerSetTest : SysuiTestCase() { - var runnableSet: ListenerSet = ListenerSet() + private val runnableSet: IListenerSet = makeRunnableListenerSet() - @Before - fun setup() { - runnableSet = ListenerSet() - } + open fun makeRunnableListenerSet(): IListenerSet = ListenerSet() @Test fun addIfAbsent_doesNotDoubleAdd() { // setup & preconditions val runnable1 = Runnable { } val runnable2 = Runnable { } - assertThat(runnableSet.toList()).isEmpty() + assertThat(runnableSet).isEmpty() // Test that an element can be added assertThat(runnableSet.addIfAbsent(runnable1)).isTrue() - assertThat(runnableSet.toList()).containsExactly(runnable1) + assertThat(runnableSet).containsExactly(runnable1) // Test that a second element can be added assertThat(runnableSet.addIfAbsent(runnable2)).isTrue() - assertThat(runnableSet.toList()).containsExactly(runnable1, runnable2) + assertThat(runnableSet).containsExactly(runnable1, runnable2) // Test that re-adding the first element does nothing and returns false assertThat(runnableSet.addIfAbsent(runnable1)).isFalse() - assertThat(runnableSet.toList()).containsExactly(runnable1, runnable2) + assertThat(runnableSet).containsExactly(runnable1, runnable2) + } + + @Test + fun isEmpty_changes() { + val runnable = Runnable { } + assertThat(runnableSet).isEmpty() + assertThat(runnableSet.isEmpty()).isTrue() + assertThat(runnableSet.isNotEmpty()).isFalse() + + assertThat(runnableSet.addIfAbsent(runnable)).isTrue() + assertThat(runnableSet).isNotEmpty() + assertThat(runnableSet.isEmpty()).isFalse() + assertThat(runnableSet.isNotEmpty()).isTrue() + + assertThat(runnableSet.remove(runnable)).isTrue() + assertThat(runnableSet).isEmpty() + assertThat(runnableSet.isEmpty()).isTrue() + assertThat(runnableSet.isNotEmpty()).isFalse() + } + + @Test + fun size_changes() { + assertThat(runnableSet).isEmpty() + assertThat(runnableSet.size).isEqualTo(0) + + assertThat(runnableSet.addIfAbsent(Runnable { })).isTrue() + assertThat(runnableSet.size).isEqualTo(1) + + assertThat(runnableSet.addIfAbsent(Runnable { })).isTrue() + assertThat(runnableSet.size).isEqualTo(2) + } + + @Test + fun contains_worksAsExpected() { + val runnable1 = Runnable { } + val runnable2 = Runnable { } + assertThat(runnableSet).isEmpty() + assertThat(runnable1 in runnableSet).isFalse() + assertThat(runnable2 in runnableSet).isFalse() + assertThat(runnableSet).doesNotContain(runnable1) + assertThat(runnableSet).doesNotContain(runnable2) + + assertThat(runnableSet.addIfAbsent(runnable1)).isTrue() + assertThat(runnable1 in runnableSet).isTrue() + assertThat(runnable2 in runnableSet).isFalse() + assertThat(runnableSet).contains(runnable1) + assertThat(runnableSet).doesNotContain(runnable2) + + assertThat(runnableSet.addIfAbsent(runnable2)).isTrue() + assertThat(runnable1 in runnableSet).isTrue() + assertThat(runnable2 in runnableSet).isTrue() + assertThat(runnableSet).contains(runnable1) + assertThat(runnableSet).contains(runnable2) + + assertThat(runnableSet.remove(runnable1)).isTrue() + assertThat(runnable1 in runnableSet).isFalse() + assertThat(runnable2 in runnableSet).isTrue() + assertThat(runnableSet).doesNotContain(runnable1) + assertThat(runnableSet).contains(runnable2) + } + + @Test + fun containsAll_worksAsExpected() { + val runnable1 = Runnable { } + val runnable2 = Runnable { } + + assertThat(runnableSet).isEmpty() + assertThat(runnableSet.containsAll(listOf())).isTrue() + assertThat(runnableSet.containsAll(listOf(runnable1))).isFalse() + assertThat(runnableSet.containsAll(listOf(runnable2))).isFalse() + assertThat(runnableSet.containsAll(listOf(runnable1, runnable2))).isFalse() + + assertThat(runnableSet.addIfAbsent(runnable1)).isTrue() + assertThat(runnableSet.containsAll(listOf())).isTrue() + assertThat(runnableSet.containsAll(listOf(runnable1))).isTrue() + assertThat(runnableSet.containsAll(listOf(runnable2))).isFalse() + assertThat(runnableSet.containsAll(listOf(runnable1, runnable2))).isFalse() + + assertThat(runnableSet.addIfAbsent(runnable2)).isTrue() + assertThat(runnableSet.containsAll(listOf())).isTrue() + assertThat(runnableSet.containsAll(listOf(runnable1))).isTrue() + assertThat(runnableSet.containsAll(listOf(runnable2))).isTrue() + assertThat(runnableSet.containsAll(listOf(runnable1, runnable2))).isTrue() + + assertThat(runnableSet.remove(runnable1)).isTrue() + assertThat(runnableSet.containsAll(listOf())).isTrue() + assertThat(runnableSet.containsAll(listOf(runnable1))).isFalse() + assertThat(runnableSet.containsAll(listOf(runnable2))).isTrue() + assertThat(runnableSet.containsAll(listOf(runnable1, runnable2))).isFalse() } @Test @@ -60,22 +145,22 @@ class ListenerSetTest : SysuiTestCase() { // setup and preconditions val runnable1 = Runnable { } val runnable2 = Runnable { } - assertThat(runnableSet.toList()).isEmpty() + assertThat(runnableSet).isEmpty() runnableSet.addIfAbsent(runnable1) runnableSet.addIfAbsent(runnable2) - assertThat(runnableSet.toList()).containsExactly(runnable1, runnable2) + assertThat(runnableSet).containsExactly(runnable1, runnable2) // Test that removing the first runnable only removes that one runnable assertThat(runnableSet.remove(runnable1)).isTrue() - assertThat(runnableSet.toList()).containsExactly(runnable2) + assertThat(runnableSet).containsExactly(runnable2) // Test that removing a non-present runnable does not error assertThat(runnableSet.remove(runnable1)).isFalse() - assertThat(runnableSet.toList()).containsExactly(runnable2) + assertThat(runnableSet).containsExactly(runnable2) // Test that removing the other runnable succeeds assertThat(runnableSet.remove(runnable2)).isTrue() - assertThat(runnableSet.toList()).isEmpty() + assertThat(runnableSet).isEmpty() } @Test @@ -92,17 +177,17 @@ class ListenerSetTest : SysuiTestCase() { val runnable2 = Runnable { runnablesCalled.add(2) } - assertThat(runnableSet.toList()).isEmpty() + assertThat(runnableSet).isEmpty() runnableSet.addIfAbsent(runnable1) runnableSet.addIfAbsent(runnable2) - assertThat(runnableSet.toList()).containsExactly(runnable1, runnable2) + assertThat(runnableSet).containsExactly(runnable1, runnable2) // Test that both runnables are called and 1 was removed for (runnable in runnableSet) { runnable.run() } assertThat(runnablesCalled).containsExactly(1, 2) - assertThat(runnableSet.toList()).containsExactly(runnable2) + assertThat(runnableSet).containsExactly(runnable2) } @Test @@ -120,16 +205,16 @@ class ListenerSetTest : SysuiTestCase() { val runnable2 = Runnable { runnablesCalled.add(2) } - assertThat(runnableSet.toList()).isEmpty() + assertThat(runnableSet).isEmpty() runnableSet.addIfAbsent(runnable1) runnableSet.addIfAbsent(runnable2) - assertThat(runnableSet.toList()).containsExactly(runnable1, runnable2) + assertThat(runnableSet).containsExactly(runnable1, runnable2) // Test that both original runnables are called and 99 was added but not called for (runnable in runnableSet) { runnable.run() } assertThat(runnablesCalled).containsExactly(1, 2) - assertThat(runnableSet.toList()).containsExactly(runnable1, runnable2, runnable99) + assertThat(runnableSet).containsExactly(runnable1, runnable2, runnable99) } } \ No newline at end of file diff --git a/packages/SystemUI/tests/src/com/android/systemui/util/NamedListenerSetTest.kt b/packages/SystemUI/tests/src/com/android/systemui/util/NamedListenerSetTest.kt new file mode 100644 index 0000000000000..c89e317a6ad99 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/util/NamedListenerSetTest.kt @@ -0,0 +1,104 @@ +/* + * Copyright (C) 2023 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.util + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.filters.SmallTest +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith + +@SmallTest +@RunWith(AndroidJUnit4::class) +class NamedListenerSetTest : ListenerSetTest() { + override fun makeRunnableListenerSet(): IListenerSet = NamedListenerSet() + + private val runnableSet = NamedListenerSet(NamedRunnable::name) + + class NamedRunnable(val name: String, private val block: () -> Unit = {}) : Runnable { + override fun run() = block() + } + + @Test + fun addIfAbsent_addsMultipleWithSameName_onlyIfInstanceIsAbsent() { + // setup & preconditions + val runnable1 = NamedRunnable("A") + val runnable2 = NamedRunnable("A") + assertThat(runnableSet).isEmpty() + + // Test that an element can be added + assertThat(runnableSet.addIfAbsent(runnable1)).isTrue() + assertThat(runnableSet).containsExactly(runnable1) + + // Test that a second element can be added, even with the same name + assertThat(runnableSet.addIfAbsent(runnable2)).isTrue() + assertThat(runnableSet).containsExactly(runnable1, runnable2) + + // Test that re-adding the first element does nothing and returns false + assertThat(runnableSet.addIfAbsent(runnable1)).isFalse() + assertThat(runnableSet).containsExactly(runnable1, runnable2) + } + + @Test + fun forEachNamed_includesCorrectNames() { + val runnable1 = NamedRunnable("A") + val runnable2 = NamedRunnable("X") + val runnable3 = NamedRunnable("X") + assertThat(runnableSet).isEmpty() + + assertThat(runnableSet.addIfAbsent(runnable1)).isTrue() + assertThat(runnableSet.toNamedPairs()) + .containsExactly( + "A" to runnable1, + ) + + assertThat(runnableSet.addIfAbsent(runnable2)).isTrue() + assertThat(runnableSet.toNamedPairs()) + .containsExactly( + "A" to runnable1, + "X" to runnable2, + ) + + assertThat(runnableSet.addIfAbsent(runnable3)).isTrue() + assertThat(runnableSet.toNamedPairs()) + .containsExactly( + "A" to runnable1, + "X" to runnable2, + "X" to runnable3, + ) + + assertThat(runnableSet.remove(runnable1)).isTrue() + assertThat(runnableSet.toNamedPairs()) + .containsExactly( + "X" to runnable2, + "X" to runnable3, + ) + + assertThat(runnableSet.remove(runnable2)).isTrue() + assertThat(runnableSet.toNamedPairs()) + .containsExactly( + "X" to runnable3, + ) + } + + /** + * This private method uses [NamedListenerSet.forEachNamed] to produce a list of pairs in order + * to validate that method. + */ + private fun NamedListenerSet.toNamedPairs() = + sequence { forEachNamed { name, listener -> yield(name to listener) } }.toList() +}