From cb10414f11e8488f757fb7040d83bff281543502 Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Thu, 7 Apr 2022 18:03:47 -0700 Subject: [PATCH] Improve Condition Monitor threading. This change modifies condition monitor to enforce all modifications to the callback listen happen on the same thread. First, the executor now is specified to be the @Main variant. Secondly, callbacks can no longer be added in the constructor. Only allowing callbacks to be introduced post construction enforces this modification is always on the main thread. Test: atest ConditionMonitorTest Fixed: 228135569 Change-Id: I376479d104e0252eb3a39d768299ce86a78e11a1 --- .../systemui/util/condition/Monitor.java | 11 ++------- .../condition/dagger/MonitorComponent.java | 3 +-- .../util/condition/ConditionMonitorTest.java | 24 ++++++++++--------- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/util/condition/Monitor.java b/packages/SystemUI/src/com/android/systemui/util/condition/Monitor.java index d3c6e9aa0da93..313d56febb750 100644 --- a/packages/SystemUI/src/com/android/systemui/util/condition/Monitor.java +++ b/packages/SystemUI/src/com/android/systemui/util/condition/Monitor.java @@ -18,6 +18,7 @@ package com.android.systemui.util.condition; import android.util.Log; +import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.statusbar.policy.CallbackController; import org.jetbrains.annotations.NotNull; @@ -60,21 +61,13 @@ public class Monitor implements CallbackController { }; @Inject - public Monitor(Executor executor, Set conditions, Set callbacks) { + public Monitor(@Main Executor executor, Set conditions) { mConditions = new HashSet<>(); mExecutor = executor; if (conditions != null) { mConditions.addAll(conditions); } - - if (callbacks == null) { - return; - } - - for (Callback callback : callbacks) { - addCallbackLocked(callback); - } } private void updateConditionMetState() { diff --git a/packages/SystemUI/src/com/android/systemui/util/condition/dagger/MonitorComponent.java b/packages/SystemUI/src/com/android/systemui/util/condition/dagger/MonitorComponent.java index fc67973fe2788..8e739d61b4f29 100644 --- a/packages/SystemUI/src/com/android/systemui/util/condition/dagger/MonitorComponent.java +++ b/packages/SystemUI/src/com/android/systemui/util/condition/dagger/MonitorComponent.java @@ -34,8 +34,7 @@ public interface MonitorComponent { */ @Subcomponent.Factory interface Factory { - MonitorComponent create(@BindsInstance Set conditions, - @BindsInstance Set callbacks); + MonitorComponent create(@BindsInstance Set conditions); } /** diff --git a/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionMonitorTest.java b/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionMonitorTest.java index 5118637ea710e..758961658f2a4 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionMonitorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionMonitorTest.java @@ -65,7 +65,7 @@ public class ConditionMonitorTest extends SysuiTestCase { mCondition3 = spy(new FakeCondition()); mConditions = new HashSet<>(Arrays.asList(mCondition1, mCondition2, mCondition3)); - mConditionMonitor = new Monitor(mExecutor, mConditions, null /*callbacks*/); + mConditionMonitor = new Monitor(mExecutor, mConditions); } @Test @@ -76,8 +76,10 @@ public class ConditionMonitorTest extends SysuiTestCase { final Monitor monitor = new Monitor( mExecutor, - new HashSet<>(Arrays.asList(overridingCondition, regularCondition)), - new HashSet<>(Arrays.asList(callback))); + new HashSet<>(Arrays.asList(overridingCondition, regularCondition))); + + monitor.addCallback(callback); + mExecutor.runAllReady(); when(overridingCondition.isOverridingCondition()).thenReturn(true); when(overridingCondition.isConditionMet()).thenReturn(true); @@ -123,8 +125,9 @@ public class ConditionMonitorTest extends SysuiTestCase { final Monitor monitor = new Monitor( mExecutor, new HashSet<>(Arrays.asList(overridingCondition, overridingCondition2, - regularCondition)), - new HashSet<>(Arrays.asList(callback))); + regularCondition))); + monitor.addCallback(callback); + mExecutor.runAllReady(); when(overridingCondition.isOverridingCondition()).thenReturn(true); when(overridingCondition.isConditionMet()).thenReturn(true); @@ -174,8 +177,8 @@ public class ConditionMonitorTest extends SysuiTestCase { mock(Monitor.Callback.class); final Condition condition = mock(Condition.class); when(condition.isConditionMet()).thenReturn(true); - final Monitor monitor = new Monitor(mExecutor, new HashSet<>(Arrays.asList(condition)), - new HashSet<>(Arrays.asList(callback1))); + final Monitor monitor = new Monitor(mExecutor, new HashSet<>(Arrays.asList(condition))); + monitor.addCallback(callback1); final Monitor.Callback callback2 = mock(Monitor.Callback.class); @@ -186,7 +189,7 @@ public class ConditionMonitorTest extends SysuiTestCase { @Test public void addCallback_noConditions_reportAllConditionsMet() { - final Monitor monitor = new Monitor(mExecutor, new HashSet<>(), null /*callbacks*/); + final Monitor monitor = new Monitor(mExecutor, new HashSet<>()); final Monitor.Callback callback = mock(Monitor.Callback.class); monitor.addCallback(callback); @@ -196,7 +199,7 @@ public class ConditionMonitorTest extends SysuiTestCase { @Test public void addCallback_withMultipleInstancesOfTheSameCallback_registerOnlyOne() { - final Monitor monitor = new Monitor(mExecutor, new HashSet<>(), null /*callbacks*/); + final Monitor monitor = new Monitor(mExecutor, new HashSet<>()); final Monitor.Callback callback = mock(Monitor.Callback.class); // Adds the same instance multiple times. @@ -212,8 +215,7 @@ public class ConditionMonitorTest extends SysuiTestCase { @Test public void removeCallback_shouldNoLongerReceiveUpdate() { final Condition condition = mock(Condition.class); - final Monitor monitor = new Monitor(mExecutor, new HashSet<>(Arrays.asList(condition)), - null); + final Monitor monitor = new Monitor(mExecutor, new HashSet<>(Arrays.asList(condition))); final Monitor.Callback callback = mock(Monitor.Callback.class); monitor.addCallback(callback);