Add ListenerSet and use in places which currently copy-on-iterate.
Fixes: 204127880
Test: atest ListenerSetTest
Merged-In: Ic1320b4f6c424322451f7def11346865bf878f99
Change-Id: Ic1320b4f6c424322451f7def11346865bf878f99
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/KeyguardBouncer.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/KeyguardBouncer.java
index b9b663c..353868b 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/KeyguardBouncer.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/KeyguardBouncer.java
@@ -45,6 +45,7 @@
import com.android.systemui.keyguard.DismissCallbackRegistry;
import com.android.systemui.shared.system.SysUiStatsLog;
import com.android.systemui.statusbar.policy.KeyguardStateController;
+import com.android.systemui.util.ListenerSet;
import java.io.PrintWriter;
import java.util.ArrayList;
@@ -84,11 +85,11 @@
private final Runnable mRemoveViewRunnable = this::removeView;
private final KeyguardBypassController mKeyguardBypassController;
private KeyguardHostViewController mKeyguardViewController;
- private final List<KeyguardResetCallback> mResetCallbacks = new ArrayList<>();
+ private final ListenerSet<KeyguardResetCallback> mResetCallbacks = new ListenerSet<>();
private final Runnable mResetRunnable = ()-> {
if (mKeyguardViewController != null) {
mKeyguardViewController.resetSecurityContainer();
- for (KeyguardResetCallback callback : new ArrayList<>(mResetCallbacks)) {
+ for (KeyguardResetCallback callback : mResetCallbacks) {
callback.onKeyguardReset();
}
}
@@ -602,7 +603,7 @@
}
public void addKeyguardResetCallback(KeyguardResetCallback callback) {
- mResetCallbacks.add(callback);
+ mResetCallbacks.addIfAbsent(callback);
}
public void removeKeyguardResetCallback(KeyguardResetCallback callback) {
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/HeadsUpManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/HeadsUpManager.java
index a8097c4..e0b0dd3 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/HeadsUpManager.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/HeadsUpManager.java
@@ -38,11 +38,10 @@
import com.android.systemui.statusbar.AlertingNotificationManager;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.InflationFlag;
+import com.android.systemui.util.ListenerSet;
import java.io.FileDescriptor;
import java.io.PrintWriter;
-import java.util.ArrayList;
-import java.util.HashSet;
/**
* A manager which handles heads up notifications which is a special mode where
@@ -52,7 +51,7 @@
private static final String TAG = "HeadsUpManager";
private static final String SETTING_HEADS_UP_SNOOZE_LENGTH_MS = "heads_up_snooze_length_ms";
- protected final HashSet<OnHeadsUpChangedListener> mListeners = new HashSet<>();
+ protected final ListenerSet<OnHeadsUpChangedListener> mListeners = new ListenerSet<>();
protected final Context mContext;
@@ -118,7 +117,7 @@
* Adds an OnHeadUpChangedListener to observe events.
*/
public void addListener(@NonNull OnHeadsUpChangedListener listener) {
- mListeners.add(listener);
+ mListeners.addIfAbsent(listener);
}
/**
@@ -158,7 +157,7 @@
NotificationPeekEvent.NOTIFICATION_PEEK, entry.getSbn().getUid(),
entry.getSbn().getPackageName(), entry.getSbn().getInstanceId());
}
- for (OnHeadsUpChangedListener listener : new ArrayList<>(mListeners)) {
+ for (OnHeadsUpChangedListener listener : mListeners) {
if (isPinned) {
listener.onHeadsUpPinned(entry);
} else {
@@ -178,7 +177,7 @@
entry.setHeadsUp(true);
setEntryPinned((HeadsUpEntry) alertEntry, shouldHeadsUpBecomePinned(entry));
EventLogTags.writeSysuiHeadsUpStatus(entry.getKey(), 1 /* visible */);
- for (OnHeadsUpChangedListener listener : new ArrayList<>(mListeners)) {
+ for (OnHeadsUpChangedListener listener : mListeners) {
listener.onHeadsUpStateChanged(entry, true);
}
}
@@ -189,7 +188,7 @@
entry.setHeadsUp(false);
setEntryPinned((HeadsUpEntry) alertEntry, false /* isPinned */);
EventLogTags.writeSysuiHeadsUpStatus(entry.getKey(), 0 /* visible */);
- for (OnHeadsUpChangedListener listener : new ArrayList<>(mListeners)) {
+ for (OnHeadsUpChangedListener listener : mListeners) {
listener.onHeadsUpStateChanged(entry, false);
}
}
@@ -207,7 +206,7 @@
if (mHasPinnedNotification) {
MetricsLogger.count(mContext, "note_peek", 1);
}
- for (OnHeadsUpChangedListener listener : new ArrayList<>(mListeners)) {
+ for (OnHeadsUpChangedListener listener : mListeners) {
listener.onHeadsUpPinnedModeChanged(hasPinnedNotification);
}
}
diff --git a/packages/SystemUI/src/com/android/systemui/util/ListenerSet.kt b/packages/SystemUI/src/com/android/systemui/util/ListenerSet.kt
new file mode 100644
index 0000000..0f4193e9
--- /dev/null
+++ b/packages/SystemUI/src/com/android/systemui/util/ListenerSet.kt
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2021 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
+
+/**
+ * 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.
+ */
+class ListenerSet<E> : Iterable<E> {
+ private val listeners: CopyOnWriteArrayList<E> = 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)
+
+ /**
+ * A thread-safe, reentrant-safe method to remove a listener.
+ */
+ fun remove(element: E): Boolean = listeners.remove(element)
+
+ /**
+ * Returns an iterator over the listeners currently in the set. Note that to ensure
+ * [ConcurrentModificationException] is never thrown, this iterator will not reflect changes
+ * made to the set after the iterator is constructed.
+ */
+ override fun iterator(): Iterator<E> = 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
new file mode 100644
index 0000000..2662da2
--- /dev/null
+++ b/packages/SystemUI/tests/src/com/android/systemui/util/ListenerSetTest.kt
@@ -0,0 +1,135 @@
+/*
+ * Copyright (C) 2021 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 android.test.suitebuilder.annotation.SmallTest
+import androidx.test.runner.AndroidJUnit4
+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() {
+
+ var runnableSet: ListenerSet<Runnable> = ListenerSet()
+
+ @Before
+ fun setup() {
+ runnableSet = ListenerSet()
+ }
+
+ @Test
+ fun addIfAbsent_doesNotDoubleAdd() {
+ // setup & preconditions
+ val runnable1 = Runnable { }
+ val runnable2 = Runnable { }
+ assertThat(runnableSet.toList()).isEmpty()
+
+ // Test that an element can be added
+ assertThat(runnableSet.addIfAbsent(runnable1)).isTrue()
+ assertThat(runnableSet.toList()).containsExactly(runnable1)
+
+ // Test that a second element can be added
+ assertThat(runnableSet.addIfAbsent(runnable2)).isTrue()
+ assertThat(runnableSet.toList()).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)
+ }
+
+ @Test
+ fun remove_removesListener() {
+ // setup and preconditions
+ val runnable1 = Runnable { }
+ val runnable2 = Runnable { }
+ assertThat(runnableSet.toList()).isEmpty()
+ runnableSet.addIfAbsent(runnable1)
+ runnableSet.addIfAbsent(runnable2)
+ assertThat(runnableSet.toList()).containsExactly(runnable1, runnable2)
+
+ // Test that removing the first runnable only removes that one runnable
+ assertThat(runnableSet.remove(runnable1)).isTrue()
+ assertThat(runnableSet.toList()).containsExactly(runnable2)
+
+ // Test that removing a non-present runnable does not error
+ assertThat(runnableSet.remove(runnable1)).isFalse()
+ assertThat(runnableSet.toList()).containsExactly(runnable2)
+
+ // Test that removing the other runnable succeeds
+ assertThat(runnableSet.remove(runnable2)).isTrue()
+ assertThat(runnableSet.toList()).isEmpty()
+ }
+
+ @Test
+ fun remove_isReentrantSafe() {
+ // Setup and preconditions
+ val runnablesCalled = mutableListOf<Int>()
+ // runnable1 is configured to remove itself
+ val runnable1 = object : Runnable {
+ override fun run() {
+ runnableSet.remove(this)
+ runnablesCalled.add(1)
+ }
+ }
+ val runnable2 = Runnable {
+ runnablesCalled.add(2)
+ }
+ assertThat(runnableSet.toList()).isEmpty()
+ runnableSet.addIfAbsent(runnable1)
+ runnableSet.addIfAbsent(runnable2)
+ assertThat(runnableSet.toList()).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)
+ }
+
+ @Test
+ fun addIfAbsent_isReentrantSafe() {
+ // Setup and preconditions
+ val runnablesCalled = mutableListOf<Int>()
+ val runnable99 = Runnable {
+ runnablesCalled.add(99)
+ }
+ // runnable1 is configured to add runnable99
+ val runnable1 = Runnable {
+ runnableSet.addIfAbsent(runnable99)
+ runnablesCalled.add(1)
+ }
+ val runnable2 = Runnable {
+ runnablesCalled.add(2)
+ }
+ assertThat(runnableSet.toList()).isEmpty()
+ runnableSet.addIfAbsent(runnable1)
+ runnableSet.addIfAbsent(runnable2)
+ assertThat(runnableSet.toList()).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)
+ }
+}
\ No newline at end of file