diff --git a/packages/SystemUI/src/com/android/systemui/util/condition/Condition.java b/packages/SystemUI/src/com/android/systemui/util/condition/Condition.java index 0bbf56caaaf1c..db35437e77b98 100644 --- a/packages/SystemUI/src/com/android/systemui/util/condition/Condition.java +++ b/packages/SystemUI/src/com/android/systemui/util/condition/Condition.java @@ -36,6 +36,7 @@ public abstract class Condition implements CallbackController> mCallbacks = new ArrayList<>(); private boolean mIsConditionMet = false; private boolean mStarted = false; + private boolean mOverriding = false; /** * Starts monitoring the condition. @@ -47,6 +48,21 @@ public abstract class Condition implements CallbackController(callback)); if (mStarted) { - callback.onConditionChanged(this, mIsConditionMet); + callback.onConditionChanged(this); return; } @@ -107,11 +123,15 @@ public abstract class Condition implements CallbackController { // Set of all conditions that need to be monitored. private final Set mConditions; - - // Map of values of each condition. - private final HashMap mConditionsMap = new HashMap<>(); + private final Executor mExecutor; // Whether all conditions have been met. private boolean mAllConditionsMet = false; @@ -52,10 +52,43 @@ public class Monitor implements CallbackController { private boolean mHaveConditionsStarted = false; // Callback for when each condition has been updated. - private final Condition.Callback mConditionCallback = (condition, isConditionMet) -> { - mConditionsMap.put(condition, isConditionMet); + private final Condition.Callback mConditionCallback = new Condition.Callback() { + @Override + public void onConditionChanged(Condition condition) { + mExecutor.execute(() -> updateConditionMetState()); + } + }; - final boolean newAllConditionsMet = !mConditionsMap.containsValue(false); + @Inject + public Monitor(Executor executor, Set conditions, Set callbacks) { + mConditions = new HashSet<>(); + mExecutor = executor; + + if (conditions != null) { + mConditions.addAll(conditions); + } + + if (callbacks == null) { + return; + } + + for (Callback callback : callbacks) { + addCallbackLocked(callback); + } + } + + private void updateConditionMetState() { + // Overriding conditions do not override each other + final Collection overridingConditions = mConditions.stream() + .filter(Condition::isOverridingCondition).collect(Collectors.toSet()); + + final Collection targetCollection = overridingConditions.isEmpty() + ? mConditions : overridingConditions; + + final boolean newAllConditionsMet = targetCollection.isEmpty() ? true : targetCollection + .stream() + .map(Condition::isConditionMet) + .allMatch(conditionMet -> conditionMet); if (newAllConditionsMet == mAllConditionsMet) { return; @@ -74,32 +107,44 @@ public class Monitor implements CallbackController { callback.onConditionsChanged(mAllConditionsMet); } } - }; - - @Inject - public Monitor(Set conditions, Set callbacks) { - mConditions = conditions; - - // If there is no condition, give green pass. - if (mConditions.isEmpty()) { - mAllConditionsMet = true; - return; - } - - // Initializes the conditions map and registers a callback for each condition. - mConditions.forEach((condition -> mConditionsMap.put(condition, false))); - - if (callbacks == null) { - return; - } - - for (Callback callback : callbacks) { - addCallback(callback); - } } - @Override - public void addCallback(@NotNull Callback callback) { + private void addConditionLocked(@NotNull Condition condition) { + mConditions.add(condition); + + if (!mHaveConditionsStarted) { + return; + } + + condition.addCallback(mConditionCallback); + updateConditionMetState(); + } + + /** + * Adds a condition for the monitor to listen to and consider when determining whether the + * overall condition state is met. + */ + public void addCondition(@NotNull Condition condition) { + mExecutor.execute(() -> addConditionLocked(condition)); + } + + /** + * Removes a condition from further consideration. + */ + public void removeCondition(@NotNull Condition condition) { + mExecutor.execute(() -> { + mConditions.remove(condition); + + if (!mHaveConditionsStarted) { + return; + } + + condition.removeCallback(mConditionCallback); + updateConditionMetState(); + }); + } + + private void addCallbackLocked(@NotNull Callback callback) { if (shouldLog()) Log.d(mTag, "adding callback"); mCallbacks.add(callback); @@ -109,36 +154,36 @@ public class Monitor implements CallbackController { if (!mHaveConditionsStarted) { if (shouldLog()) Log.d(mTag, "starting all conditions"); mConditions.forEach(condition -> condition.addCallback(mConditionCallback)); + updateConditionMetState(); mHaveConditionsStarted = true; } } @Override - public void removeCallback(@NotNull Callback callback) { - if (shouldLog()) Log.d(mTag, "removing callback"); - final Iterator iterator = mCallbacks.iterator(); - while (iterator.hasNext()) { - final Callback cb = iterator.next(); - if (cb == null || cb == callback) { - iterator.remove(); - } - } - - if (mCallbacks.isEmpty() && mHaveConditionsStarted) { - if (shouldLog()) Log.d(mTag, "stopping all conditions"); - mConditions.forEach(condition -> condition.removeCallback(mConditionCallback)); - - mAllConditionsMet = false; - mHaveConditionsStarted = false; - } + public void addCallback(@NotNull Callback callback) { + mExecutor.execute(() -> addCallbackLocked(callback)); } - /** - * Force updates each condition to the value provided. - */ - @VisibleForTesting - public void overrideAllConditionsMet(boolean value) { - mConditions.forEach(condition -> condition.updateCondition(value)); + @Override + public void removeCallback(@NotNull Callback callback) { + mExecutor.execute(() -> { + if (shouldLog()) Log.d(mTag, "removing callback"); + final Iterator iterator = mCallbacks.iterator(); + while (iterator.hasNext()) { + final Callback cb = iterator.next(); + if (cb == null || cb == callback) { + iterator.remove(); + } + } + + if (mCallbacks.isEmpty() && mHaveConditionsStarted) { + if (shouldLog()) Log.d(mTag, "stopping all conditions"); + mConditions.forEach(condition -> condition.removeCallback(mConditionCallback)); + + mAllConditionsMet = false; + mHaveConditionsStarted = false; + } + }); } private boolean shouldLog() { diff --git a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalSettingConditionTest.java b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalSettingConditionTest.java index 2d52c42fa75fd..c5b1a1d8bac5f 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalSettingConditionTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalSettingConditionTest.java @@ -16,7 +16,8 @@ package com.android.systemui.communal; -import static org.mockito.ArgumentMatchers.anyBoolean; +import static com.google.common.truth.Truth.assertThat; + import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.mock; @@ -59,7 +60,8 @@ public class CommunalSettingConditionTest extends SysuiTestCase { final Condition.Callback callback = mock(Condition.Callback.class); mCondition.addCallback(callback); - verify(callback).onConditionChanged(mCondition, true); + verify(callback).onConditionChanged(mCondition); + assertThat(mCondition.isConditionMet()).isTrue(); } @Test @@ -68,7 +70,7 @@ public class CommunalSettingConditionTest extends SysuiTestCase { final Condition.Callback callback = mock(Condition.Callback.class); mCondition.addCallback(callback); - verify(callback, never()).onConditionChanged(eq(mCondition), anyBoolean()); + verify(callback, never()).onConditionChanged(eq(mCondition)); } @Test @@ -80,7 +82,8 @@ public class CommunalSettingConditionTest extends SysuiTestCase { clearInvocations(callback); updateCommunalSetting(true); - verify(callback).onConditionChanged(mCondition, true); + verify(callback).onConditionChanged(mCondition); + assertThat(mCondition.isConditionMet()).isTrue(); } @Test @@ -92,7 +95,8 @@ public class CommunalSettingConditionTest extends SysuiTestCase { clearInvocations(callback); updateCommunalSetting(false); - verify(callback).onConditionChanged(mCondition, false); + verify(callback).onConditionChanged(mCondition); + assertThat(mCondition.isConditionMet()).isFalse(); } @Test @@ -104,7 +108,8 @@ public class CommunalSettingConditionTest extends SysuiTestCase { clearInvocations(callback); updateCommunalSetting(true); - verify(callback, never()).onConditionChanged(mCondition, true); + verify(callback, never()).onConditionChanged(mCondition); + assertThat(mCondition.isConditionMet()).isTrue(); } private void updateCommunalSetting(boolean value) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalTrustedNetworkConditionTest.java b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalTrustedNetworkConditionTest.java index 61a5126fb139a..500205c49ad28 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalTrustedNetworkConditionTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalTrustedNetworkConditionTest.java @@ -16,7 +16,8 @@ package com.android.systemui.communal; -import static org.mockito.ArgumentMatchers.anyBoolean; +import static com.google.common.truth.Truth.assertThat; + import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; import static org.mockito.Mockito.clearInvocations; @@ -49,6 +50,7 @@ import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.MockitoAnnotations; @SmallTest @@ -89,7 +91,8 @@ public class CommunalTrustedNetworkConditionTest extends SysuiTestCase { networkCallback.onCapabilitiesChanged(network, fakeNetworkCapabilities(mTrustedWifi1)); // Verifies that the callback is triggered. - verify(callback).onConditionChanged(mCondition, true); + verify(callback).onConditionChanged(mCondition); + assertThat(mCondition.isConditionMet()).isTrue(); } @Test @@ -110,7 +113,7 @@ public class CommunalTrustedNetworkConditionTest extends SysuiTestCase { networkCallback.onCapabilitiesChanged(network, fakeNetworkCapabilities(mTrustedWifi2)); // Verifies that the callback is not triggered. - verify(callback, never()).onConditionChanged(eq(mCondition), anyBoolean()); + verify(callback, never()).onConditionChanged(eq(mCondition)); } @Test @@ -126,11 +129,13 @@ public class CommunalTrustedNetworkConditionTest extends SysuiTestCase { networkCallback.onAvailable(network); networkCallback.onCapabilitiesChanged(network, fakeNetworkCapabilities(mTrustedWifi1)); + Mockito.clearInvocations(callback); // Connected to non-trusted Wi-Fi network. networkCallback.onCapabilitiesChanged(network, fakeNetworkCapabilities("random-wifi")); // Verifies that the callback is triggered. - verify(callback).onConditionChanged(mCondition, false); + verify(callback).onConditionChanged(mCondition); + assertThat(mCondition.isConditionMet()).isFalse(); } @Test @@ -151,7 +156,8 @@ public class CommunalTrustedNetworkConditionTest extends SysuiTestCase { networkCallback.onLost(network); // Verifies that the callback is triggered. - verify(callback).onConditionChanged(mCondition, false); + verify(callback).onConditionChanged(mCondition); + assertThat(mCondition.isConditionMet()).isFalse(); } // Captures and returns the network callback, assuming it is registered with the connectivity 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 d6454490e62a0..dff77f389faf7 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 @@ -17,6 +17,7 @@ package com.android.systemui.util.condition; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.anyBoolean; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.mock; @@ -24,16 +25,21 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import android.testing.AndroidTestingRunner; import androidx.test.filters.SmallTest; import com.android.systemui.SysuiTestCase; +import com.android.systemui.util.concurrency.FakeExecutor; +import com.android.systemui.util.time.FakeSystemClock; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import java.util.Arrays; @@ -46,6 +52,7 @@ public class ConditionMonitorTest extends SysuiTestCase { private FakeCondition mCondition2; private FakeCondition mCondition3; private HashSet mConditions; + private FakeExecutor mExecutor = new FakeExecutor(new FakeSystemClock()); private Monitor mConditionMonitor; @@ -58,7 +65,83 @@ public class ConditionMonitorTest extends SysuiTestCase { mCondition3 = spy(new FakeCondition()); mConditions = new HashSet<>(Arrays.asList(mCondition1, mCondition2, mCondition3)); - mConditionMonitor = new Monitor(mConditions, null /*callbacks*/); + mConditionMonitor = new Monitor(mExecutor, mConditions, null /*callbacks*/); + } + + @Test + public void testOverridingCondition() { + final Condition overridingCondition = Mockito.mock(Condition.class); + final Condition regularCondition = Mockito.mock(Condition.class); + final Monitor.Callback callback = Mockito.mock(Monitor.Callback.class); + + final Monitor monitor = new Monitor( + mExecutor, + new HashSet<>(Arrays.asList(overridingCondition, regularCondition)), + new HashSet<>(Arrays.asList(callback))); + + when(overridingCondition.isOverridingCondition()).thenReturn(true); + when(overridingCondition.isConditionMet()).thenReturn(true); + when(regularCondition.isConditionMet()).thenReturn(false); + + final ArgumentCaptor mCallbackCaptor = + ArgumentCaptor.forClass(Condition.Callback.class); + + verify(overridingCondition).addCallback(mCallbackCaptor.capture()); + + mCallbackCaptor.getValue().onConditionChanged(overridingCondition); + mExecutor.runAllReady(); + + verify(callback).onConditionsChanged(eq(true)); + Mockito.clearInvocations(callback); + + when(regularCondition.isConditionMet()).thenReturn(true); + when(overridingCondition.isConditionMet()).thenReturn(false); + + mCallbackCaptor.getValue().onConditionChanged(overridingCondition); + mExecutor.runAllReady(); + + verify(callback).onConditionsChanged(eq(false)); + + clearInvocations(callback); + monitor.removeCondition(overridingCondition); + mExecutor.runAllReady(); + + verify(callback).onConditionsChanged(eq(true)); + } + + /** + * Ensures that when multiple overriding conditions are present, it is the aggregate of those + * conditions that are considered. + */ + @Test + public void testMultipleOverridingConditions() { + final Condition overridingCondition = Mockito.mock(Condition.class); + final Condition overridingCondition2 = Mockito.mock(Condition.class); + final Condition regularCondition = Mockito.mock(Condition.class); + final Monitor.Callback callback = Mockito.mock(Monitor.Callback.class); + + final Monitor monitor = new Monitor( + mExecutor, + new HashSet<>(Arrays.asList(overridingCondition, overridingCondition2, + regularCondition)), + new HashSet<>(Arrays.asList(callback))); + + when(overridingCondition.isOverridingCondition()).thenReturn(true); + when(overridingCondition.isConditionMet()).thenReturn(true); + when(overridingCondition2.isOverridingCondition()).thenReturn(true); + when(overridingCondition.isConditionMet()).thenReturn(false); + when(regularCondition.isConditionMet()).thenReturn(true); + + final ArgumentCaptor mCallbackCaptor = + ArgumentCaptor.forClass(Condition.Callback.class); + + verify(overridingCondition).addCallback(mCallbackCaptor.capture()); + + mCallbackCaptor.getValue().onConditionChanged(overridingCondition); + mExecutor.runAllReady(); + + verify(callback).onConditionsChanged(eq(false)); + Mockito.clearInvocations(callback); } @Test @@ -66,11 +149,13 @@ public class ConditionMonitorTest extends SysuiTestCase { final Monitor.Callback callback1 = mock(Monitor.Callback.class); mConditionMonitor.addCallback(callback1); + mExecutor.runAllReady(); mConditions.forEach(condition -> verify(condition).addCallback(any())); final Monitor.Callback callback2 = mock(Monitor.Callback.class); mConditionMonitor.addCallback(callback2); + mExecutor.runAllReady(); mConditions.forEach(condition -> verify(condition, times(1)).addCallback(any())); } @@ -79,6 +164,7 @@ public class ConditionMonitorTest extends SysuiTestCase { final Monitor.Callback callback = mock(Monitor.Callback.class); mConditionMonitor.addCallback(callback); + mExecutor.runAllReady(); verify(callback).onConditionsChanged(false); } @@ -86,38 +172,53 @@ public class ConditionMonitorTest extends SysuiTestCase { public void addCallback_addSecondCallback_reportWithExistingValue() { final Monitor.Callback callback1 = mock(Monitor.Callback.class); - mConditionMonitor.addCallback(callback1); - - mConditionMonitor.overrideAllConditionsMet(true); + 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.Callback callback2 = mock(Monitor.Callback.class); - mConditionMonitor.addCallback(callback2); - verify(callback2).onConditionsChanged(true); + monitor.addCallback(callback2); + mExecutor.runAllReady(); + verify(callback2).onConditionsChanged(eq(true)); } @Test public void addCallback_noConditions_reportAllConditionsMet() { - final Monitor monitor = new Monitor(new HashSet<>(), null /*callbacks*/); + final Monitor monitor = new Monitor(mExecutor, new HashSet<>(), null /*callbacks*/); final Monitor.Callback callback = mock(Monitor.Callback.class); monitor.addCallback(callback); - + mExecutor.runAllReady(); verify(callback).onConditionsChanged(true); } @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.Callback callback = mock(Monitor.Callback.class); - mConditionMonitor.addCallback(callback); + monitor.addCallback(callback); + monitor.removeCallback(callback); + mExecutor.runAllReady(); clearInvocations(callback); - mConditionMonitor.removeCallback(callback); - mConditionMonitor.overrideAllConditionsMet(true); + final ArgumentCaptor conditionCallbackCaptor = + ArgumentCaptor.forClass(Condition.Callback.class); + verify(condition).addCallback(conditionCallbackCaptor.capture()); + final Condition.Callback conditionCallback = conditionCallbackCaptor.getValue(); + + when(condition.isConditionMet()).thenReturn(true); + conditionCallback.onConditionChanged(condition); + mExecutor.runAllReady(); verify(callback, never()).onConditionsChanged(true); - mConditionMonitor.overrideAllConditionsMet(false); + when(condition.isConditionMet()).thenReturn(false); + conditionCallback.onConditionChanged(condition); + mExecutor.runAllReady(); verify(callback, never()).onConditionsChanged(false); } @@ -131,9 +232,11 @@ public class ConditionMonitorTest extends SysuiTestCase { mConditionMonitor.addCallback(callback2); mConditionMonitor.removeCallback(callback1); + mExecutor.runAllReady(); mConditions.forEach(condition -> verify(condition, never()).removeCallback(any())); mConditionMonitor.removeCallback(callback2); + mExecutor.runAllReady(); mConditions.forEach(condition -> verify(condition).removeCallback(any())); } @@ -147,6 +250,7 @@ public class ConditionMonitorTest extends SysuiTestCase { mCondition1.fakeUpdateCondition(true); mCondition2.fakeUpdateCondition(true); mCondition3.fakeUpdateCondition(true); + mExecutor.runAllReady(); verify(callback).onConditionsChanged(true); } @@ -163,6 +267,7 @@ public class ConditionMonitorTest extends SysuiTestCase { clearInvocations(callback); mCondition1.fakeUpdateCondition(false); + mExecutor.runAllReady(); verify(callback).onConditionsChanged(false); } @@ -171,16 +276,20 @@ public class ConditionMonitorTest extends SysuiTestCase { final Monitor.Callback callback = mock(Monitor.Callback.class); mConditionMonitor.addCallback(callback); + mExecutor.runAllReady(); verify(callback).onConditionsChanged(false); clearInvocations(callback); mCondition1.fakeUpdateCondition(true); + mExecutor.runAllReady(); verify(callback, never()).onConditionsChanged(anyBoolean()); mCondition2.fakeUpdateCondition(true); + mExecutor.runAllReady(); verify(callback, never()).onConditionsChanged(anyBoolean()); mCondition3.fakeUpdateCondition(true); + mExecutor.runAllReady(); verify(callback).onConditionsChanged(true); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionTest.java b/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionTest.java index 7fc6b51bf2a66..9e0f863acc1a9 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionTest.java @@ -16,7 +16,8 @@ package com.android.systemui.util.condition; -import static org.mockito.ArgumentMatchers.anyBoolean; +import static com.google.common.truth.Truth.assertThat; + import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -73,7 +74,8 @@ public class ConditionTest extends SysuiTestCase { final Condition.Callback callback2 = mock(Condition.Callback.class); mCondition.addCallback(callback2); - verify(callback2).onConditionChanged(mCondition, true); + verify(callback2).onConditionChanged(mCondition); + assertThat(mCondition.isConditionMet()).isTrue(); } @Test @@ -94,7 +96,8 @@ public class ConditionTest extends SysuiTestCase { mCondition.addCallback(callback); mCondition.fakeUpdateCondition(true); - verify(callback).onConditionChanged(eq(mCondition), eq(true)); + verify(callback).onConditionChanged(eq(mCondition)); + assertThat(mCondition.isConditionMet()).isTrue(); } @Test @@ -105,7 +108,8 @@ public class ConditionTest extends SysuiTestCase { mCondition.addCallback(callback); mCondition.fakeUpdateCondition(false); - verify(callback).onConditionChanged(eq(mCondition), eq(false)); + verify(callback).onConditionChanged(eq(mCondition)); + assertThat(mCondition.isConditionMet()).isFalse(); } @Test @@ -116,7 +120,7 @@ public class ConditionTest extends SysuiTestCase { mCondition.addCallback(callback); mCondition.fakeUpdateCondition(true); - verify(callback, never()).onConditionChanged(eq(mCondition), anyBoolean()); + verify(callback, never()).onConditionChanged(eq(mCondition)); } @Test @@ -127,6 +131,6 @@ public class ConditionTest extends SysuiTestCase { mCondition.addCallback(callback); mCondition.fakeUpdateCondition(false); - verify(callback, never()).onConditionChanged(eq(mCondition), anyBoolean()); + verify(callback, never()).onConditionChanged(eq(mCondition)); } }