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 a090ac35070e0..7abe9bf8a242a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/KeyguardBouncer.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/KeyguardBouncer.java @@ -44,6 +44,7 @@ import com.android.systemui.dagger.qualifiers.RootView; 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; @@ -83,11 +84,11 @@ public class KeyguardBouncer { private final Runnable mRemoveViewRunnable = this::removeView; private final KeyguardBypassController mKeyguardBypassController; private KeyguardHostViewController mKeyguardViewController; - private final List mResetCallbacks = new ArrayList<>(); + private final ListenerSet 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 class KeyguardBouncer { } 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 a8097c4d74b04..e0b0dd36ccd93 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.R; 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 @@ public abstract class HeadsUpManager extends AlertingNotificationManager { private static final String TAG = "HeadsUpManager"; private static final String SETTING_HEADS_UP_SNOOZE_LENGTH_MS = "heads_up_snooze_length_ms"; - protected final HashSet mListeners = new HashSet<>(); + protected final ListenerSet mListeners = new ListenerSet<>(); protected final Context mContext; @@ -118,7 +117,7 @@ public abstract class HeadsUpManager extends AlertingNotificationManager { * Adds an OnHeadUpChangedListener to observe events. */ public void addListener(@NonNull OnHeadsUpChangedListener listener) { - mListeners.add(listener); + mListeners.addIfAbsent(listener); } /** @@ -158,7 +157,7 @@ public abstract class HeadsUpManager extends AlertingNotificationManager { 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 @@ public abstract class HeadsUpManager extends AlertingNotificationManager { 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 @@ public abstract class HeadsUpManager extends AlertingNotificationManager { 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 @@ public abstract class HeadsUpManager extends AlertingNotificationManager { 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 0000000000000..0f4193e941968 --- /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 : Iterable { + private val listeners: CopyOnWriteArrayList = 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 = 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 0000000000000..2662da201460c --- /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 = 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() + // 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() + 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