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
This commit is contained in:
Bryce Lee
2022-04-07 18:03:47 -07:00
parent 107e9d5042
commit cb10414f11
3 changed files with 16 additions and 22 deletions

View File

@@ -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<Monitor.Callback> {
};
@Inject
public Monitor(Executor executor, Set<Condition> conditions, Set<Callback> callbacks) {
public Monitor(@Main Executor executor, Set<Condition> 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() {

View File

@@ -34,8 +34,7 @@ public interface MonitorComponent {
*/
@Subcomponent.Factory
interface Factory {
MonitorComponent create(@BindsInstance Set<Condition> conditions,
@BindsInstance Set<Monitor.Callback> callbacks);
MonitorComponent create(@BindsInstance Set<Condition> conditions);
}
/**

View File

@@ -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);