diff options
4 files changed, 227 insertions, 210 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 313d56febb75..4e9030f8045c 100644 --- a/packages/SystemUI/src/com/android/systemui/util/condition/Monitor.java +++ b/packages/SystemUI/src/com/android/systemui/util/condition/Monitor.java @@ -16,17 +16,17 @@ package com.android.systemui.util.condition; +import android.util.ArraySet; import android.util.Log; import com.android.systemui.dagger.qualifiers.Main; -import com.android.systemui.statusbar.policy.CallbackController; import org.jetbrains.annotations.NotNull; import java.util.ArrayList; import java.util.Collection; -import java.util.HashSet; -import java.util.Iterator; +import java.util.Collections; +import java.util.HashMap; import java.util.Set; import java.util.concurrent.Executor; import java.util.stream.Collectors; @@ -37,154 +37,200 @@ import javax.inject.Inject; * {@link Monitor} takes in a set of conditions, monitors whether all of them have * been fulfilled, and informs any registered listeners. */ -public class Monitor implements CallbackController<Monitor.Callback> { +public class Monitor { private final String mTag = getClass().getSimpleName(); + private final Executor mExecutor; - private final ArrayList<Callback> mCallbacks = new ArrayList<>(); + private final HashMap<Condition, ArraySet<Subscription.Token>> mConditions = new HashMap<>(); + private final HashMap<Subscription.Token, SubscriptionState> mSubscriptions = new HashMap<>(); - // Set of all conditions that need to be monitored. - private final Set<Condition> mConditions; - private final Executor mExecutor; + private static class SubscriptionState { + private final Subscription mSubscription; + private Boolean mAllConditionsMet; - // Whether all conditions have been met. - private boolean mAllConditionsMet = false; + SubscriptionState(Subscription subscription) { + mSubscription = subscription; + } + + public Set<Condition> getConditions() { + return mSubscription.mConditions; + } - // Whether the monitor has started listening for all the conditions. - private boolean mHaveConditionsStarted = false; + public void update() { + // Overriding conditions do not override each other + final Collection<Condition> overridingConditions = mSubscription.mConditions.stream() + .filter(Condition::isOverridingCondition).collect(Collectors.toSet()); + + final Collection<Condition> targetCollection = overridingConditions.isEmpty() + ? mSubscription.mConditions : overridingConditions; + + final boolean newAllConditionsMet = targetCollection.isEmpty() ? true : targetCollection + .stream() + .map(Condition::isConditionMet) + .allMatch(conditionMet -> conditionMet); + + if (mAllConditionsMet != null && newAllConditionsMet == mAllConditionsMet) { + return; + } + + mAllConditionsMet = newAllConditionsMet; + mSubscription.mCallback.onConditionsChanged(mAllConditionsMet); + } + } // Callback for when each condition has been updated. private final Condition.Callback mConditionCallback = new Condition.Callback() { @Override public void onConditionChanged(Condition condition) { - mExecutor.execute(() -> updateConditionMetState()); + mExecutor.execute(() -> updateConditionMetState(condition)); } }; @Inject - public Monitor(@Main Executor executor, Set<Condition> conditions) { - mConditions = new HashSet<>(); + public Monitor(@Main Executor executor) { mExecutor = executor; - - if (conditions != null) { - mConditions.addAll(conditions); - } } - private void updateConditionMetState() { - // Overriding conditions do not override each other - final Collection<Condition> overridingConditions = mConditions.stream() - .filter(Condition::isOverridingCondition).collect(Collectors.toSet()); - - final Collection<Condition> targetCollection = overridingConditions.isEmpty() - ? mConditions : overridingConditions; - - final boolean newAllConditionsMet = targetCollection.isEmpty() ? true : targetCollection - .stream() - .map(Condition::isConditionMet) - .allMatch(conditionMet -> conditionMet); + private void updateConditionMetState(Condition condition) { + mConditions.get(condition).stream().forEach(token -> mSubscriptions.get(token).update()); + } - if (newAllConditionsMet == mAllConditionsMet) { - return; - } + /** + * Registers a callback and the set of conditions to trigger it. + * @param subscription A {@link Subscription} detailing the desired conditions and callback. + * @return A {@link Subscription.Token} that can be used to remove the subscription. + */ + public Subscription.Token addSubscription(@NotNull Subscription subscription) { + final Subscription.Token token = new Subscription.Token(); + final SubscriptionState state = new SubscriptionState(subscription); - if (shouldLog()) Log.d(mTag, "all conditions met: " + newAllConditionsMet); - mAllConditionsMet = newAllConditionsMet; - - // Updates all callbacks. - final Iterator<Callback> iterator = mCallbacks.iterator(); - while (iterator.hasNext()) { - final Callback callback = iterator.next(); - if (callback == null) { - iterator.remove(); - } else { - callback.onConditionsChanged(mAllConditionsMet); - } - } - } + mExecutor.execute(() -> { + mSubscriptions.put(token, state); - private void addConditionLocked(@NotNull Condition condition) { - mConditions.add(condition); + // Add and associate conditions. + subscription.getConditions().stream().forEach(condition -> { + if (!mConditions.containsKey(condition)) { + mConditions.put(condition, new ArraySet<>()); + condition.addCallback(mConditionCallback); + } - if (!mHaveConditionsStarted) { - return; - } + mConditions.get(condition).add(token); + }); - condition.addCallback(mConditionCallback); - updateConditionMetState(); - } + // Update subscription state. + state.update(); - /** - * 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)); + }); + return token; } /** - * Removes a condition from further consideration. + * Removes a subscription from participating in future callbacks. + * @param token The {@link Subscription.Token} returned when the {@link Subscription} was + * originally added. */ - public void removeCondition(@NotNull Condition condition) { + public void removeSubscription(@NotNull Subscription.Token token) { mExecutor.execute(() -> { - mConditions.remove(condition); - - if (!mHaveConditionsStarted) { + if (shouldLog()) Log.d(mTag, "removing callback"); + if (!mSubscriptions.containsKey(token)) { + Log.e(mTag, "subscription not present:" + token); return; } - condition.removeCallback(mConditionCallback); - updateConditionMetState(); + mSubscriptions.remove(token).getConditions().forEach(condition -> { + if (!mConditions.containsKey(condition)) { + Log.e(mTag, "condition not present:" + condition); + return; + + } + final Set<Subscription.Token> conditionSubscriptions = mConditions.get(condition); + + conditionSubscriptions.remove(token); + if (conditionSubscriptions.isEmpty()) { + condition.removeCallback(mConditionCallback); + mConditions.remove(condition); + } + }); }); } - private void addCallbackLocked(@NotNull Callback callback) { - if (mCallbacks.contains(callback)) { - return; - } + private boolean shouldLog() { + return Log.isLoggable(mTag, Log.DEBUG); + } - if (shouldLog()) Log.d(mTag, "adding callback"); - mCallbacks.add(callback); + /** + * A {@link Subscription} represents a set of conditions and a callback that is informed when + * these conditions change. + */ + public static class Subscription { + private final Set<Condition> mConditions; + private final Callback mCallback; + + /** */ + public Subscription(Set<Condition> conditions, Callback callback) { + this.mConditions = Collections.unmodifiableSet(conditions); + this.mCallback = callback; + } - // Updates the callback immediately. - callback.onConditionsChanged(mAllConditionsMet); + public Set<Condition> getConditions() { + return mConditions; + } - if (!mHaveConditionsStarted) { - if (shouldLog()) Log.d(mTag, "starting all conditions"); - mConditions.forEach(condition -> condition.addCallback(mConditionCallback)); - updateConditionMetState(); - mHaveConditionsStarted = true; + public Callback getCallback() { + return mCallback; } - } - @Override - public void addCallback(@NotNull Callback callback) { - mExecutor.execute(() -> addCallbackLocked(callback)); - } + /** + * A {@link Token} is an identifier that is associated with a {@link Subscription} which is + * registered with a {@link Monitor}. + */ + public static class Token { + } - @Override - public void removeCallback(@NotNull Callback callback) { - mExecutor.execute(() -> { - if (shouldLog()) Log.d(mTag, "removing callback"); - final Iterator<Callback> iterator = mCallbacks.iterator(); - while (iterator.hasNext()) { - final Callback cb = iterator.next(); - if (cb == null || cb == callback) { - iterator.remove(); - } + /** + * {@link Builder} is a helper class for constructing a {@link Subscription}. + */ + public static class Builder { + private final Callback mCallback; + private final ArraySet<Condition> mConditions; + + /** + * Default constructor specifying the {@link Callback} for the {@link Subscription}. + * @param callback + */ + public Builder(Callback callback) { + mCallback = callback; + mConditions = new ArraySet<>(); } - if (mCallbacks.isEmpty() && mHaveConditionsStarted) { - if (shouldLog()) Log.d(mTag, "stopping all conditions"); - mConditions.forEach(condition -> condition.removeCallback(mConditionCallback)); + /** + * Adds a {@link Condition} to be associated with the {@link Subscription}. + * @param condition + * @return The updated {@link Builder}. + */ + public Builder addCondition(Condition condition) { + mConditions.add(condition); + return this; + } - mAllConditionsMet = false; - mHaveConditionsStarted = false; + /** + * Adds a set of {@link Condition} to be associated with the {@link Subscription}. + * @param condition + * @return The updated {@link Builder}. + */ + public Builder addConditions(Set<Condition> condition) { + mConditions.addAll(condition); + return this; } - }); - } - private boolean shouldLog() { - return Log.isLoggable(mTag, Log.DEBUG); + /** + * Builds the {@link Subscription}. + * @return The resulting {@link Subscription}. + */ + public Subscription build() { + return new Subscription(mConditions, mCallback); + } + } } /** @@ -192,6 +238,13 @@ public class Monitor implements CallbackController<Monitor.Callback> { */ public interface Callback { /** + * Returns the conditions associated with this callback. + */ + default ArrayList<Condition> getConditions() { + return new ArrayList<>(); + } + + /** * Triggered when the fulfillment of all conditions have been met. * * @param allConditionsMet True if all conditions have been fulfilled. False if none or 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 deleted file mode 100644 index 8e739d61b4f2..000000000000 --- a/packages/SystemUI/src/com/android/systemui/util/condition/dagger/MonitorComponent.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * 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.condition.dagger; - -import com.android.systemui.util.condition.Condition; -import com.android.systemui.util.condition.Monitor; - -import java.util.Set; - -import dagger.BindsInstance; -import dagger.Subcomponent; - -/** - * Component for {@link Monitor}. - */ -@Subcomponent -public interface MonitorComponent { - /** - * Factory for {@link MonitorComponent}. - */ - @Subcomponent.Factory - interface Factory { - MonitorComponent create(@BindsInstance Set<Condition> conditions); - } - - /** - * Provides {@link Monitor}. - * @return - */ - Monitor getMonitor(); -} diff --git a/packages/SystemUI/src/com/android/systemui/util/dagger/UtilModule.java b/packages/SystemUI/src/com/android/systemui/util/dagger/UtilModule.java index 7892d6eec98d..981bf01164e3 100644 --- a/packages/SystemUI/src/com/android/systemui/util/dagger/UtilModule.java +++ b/packages/SystemUI/src/com/android/systemui/util/dagger/UtilModule.java @@ -18,7 +18,6 @@ package com.android.systemui.util.dagger; import com.android.systemui.util.RingerModeTracker; import com.android.systemui.util.RingerModeTrackerImpl; -import com.android.systemui.util.condition.dagger.MonitorComponent; import com.android.systemui.util.wrapper.UtilWrapperModule; import dagger.Binds; @@ -27,9 +26,6 @@ import dagger.Module; /** Dagger Module for code in the util package. */ @Module(includes = { UtilWrapperModule.class - }, - subcomponents = { - MonitorComponent.class, }) public interface UtilModule { /** */ 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 758961658f2a..1e35b0f0e68a 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,12 @@ public class ConditionMonitorTest extends SysuiTestCase { mCondition3 = spy(new FakeCondition()); mConditions = new HashSet<>(Arrays.asList(mCondition1, mCondition2, mCondition3)); - mConditionMonitor = new Monitor(mExecutor, mConditions); + mConditionMonitor = new Monitor(mExecutor); + } + + public Monitor.Subscription.Builder getDefaultBuilder(Monitor.Callback callback) { + return new Monitor.Subscription.Builder(callback) + .addConditions(mConditions); } @Test @@ -74,11 +79,19 @@ public class ConditionMonitorTest extends SysuiTestCase { 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))); + final Monitor.Callback referenceCallback = Mockito.mock(Monitor.Callback.class); + + final Monitor monitor = new Monitor(mExecutor); + + monitor.addSubscription(getDefaultBuilder(callback) + .addCondition(overridingCondition) + .addCondition(regularCondition) + .build()); + + monitor.addSubscription(getDefaultBuilder(referenceCallback) + .addCondition(regularCondition) + .build()); - monitor.addCallback(callback); mExecutor.runAllReady(); when(overridingCondition.isOverridingCondition()).thenReturn(true); @@ -94,7 +107,9 @@ public class ConditionMonitorTest extends SysuiTestCase { mExecutor.runAllReady(); verify(callback).onConditionsChanged(eq(true)); + verify(referenceCallback).onConditionsChanged(eq(false)); Mockito.clearInvocations(callback); + Mockito.clearInvocations(referenceCallback); when(regularCondition.isConditionMet()).thenReturn(true); when(overridingCondition.isConditionMet()).thenReturn(false); @@ -103,12 +118,7 @@ public class ConditionMonitorTest extends SysuiTestCase { mExecutor.runAllReady(); verify(callback).onConditionsChanged(eq(false)); - - clearInvocations(callback); - monitor.removeCondition(overridingCondition); - mExecutor.runAllReady(); - - verify(callback).onConditionsChanged(eq(true)); + verify(referenceCallback, never()).onConditionsChanged(anyBoolean()); } /** @@ -122,11 +132,13 @@ public class ConditionMonitorTest extends SysuiTestCase { 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))); - monitor.addCallback(callback); + final Monitor monitor = new Monitor(mExecutor); + + monitor.addSubscription(getDefaultBuilder(callback) + .addCondition(overridingCondition) + .addCondition(overridingCondition2) + .build()); + mExecutor.runAllReady(); when(overridingCondition.isOverridingCondition()).thenReturn(true); @@ -151,13 +163,13 @@ public class ConditionMonitorTest extends SysuiTestCase { public void addCallback_addFirstCallback_addCallbackToAllConditions() { final Monitor.Callback callback1 = mock(Monitor.Callback.class); - mConditionMonitor.addCallback(callback1); + mConditionMonitor.addSubscription(getDefaultBuilder(callback1).build()); mExecutor.runAllReady(); mConditions.forEach(condition -> verify(condition).addCallback(any())); final Monitor.Callback callback2 = mock(Monitor.Callback.class); - mConditionMonitor.addCallback(callback2); + mConditionMonitor.addSubscription(getDefaultBuilder(callback2).build()); mExecutor.runAllReady(); mConditions.forEach(condition -> verify(condition, times(1)).addCallback(any())); } @@ -166,7 +178,7 @@ public class ConditionMonitorTest extends SysuiTestCase { public void addCallback_addFirstCallback_reportWithDefaultValue() { final Monitor.Callback callback = mock(Monitor.Callback.class); - mConditionMonitor.addCallback(callback); + mConditionMonitor.addSubscription(getDefaultBuilder(callback).build()); mExecutor.runAllReady(); verify(callback).onConditionsChanged(false); } @@ -177,66 +189,65 @@ 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))); - monitor.addCallback(callback1); + final Monitor monitor = new Monitor(mExecutor); + monitor.addSubscription(new Monitor.Subscription.Builder(callback1) + .addCondition(condition) + .build()); final Monitor.Callback callback2 = mock(Monitor.Callback.class); - monitor.addCallback(callback2); + monitor.addSubscription(new Monitor.Subscription.Builder(callback2) + .addCondition(condition) + .build()); mExecutor.runAllReady(); verify(callback2).onConditionsChanged(eq(true)); } @Test public void addCallback_noConditions_reportAllConditionsMet() { - final Monitor monitor = new Monitor(mExecutor, new HashSet<>()); + final Monitor monitor = new Monitor(mExecutor); final Monitor.Callback callback = mock(Monitor.Callback.class); - monitor.addCallback(callback); + monitor.addSubscription(new Monitor.Subscription.Builder(callback).build()); mExecutor.runAllReady(); verify(callback).onConditionsChanged(true); } @Test - public void addCallback_withMultipleInstancesOfTheSameCallback_registerOnlyOne() { - final Monitor monitor = new Monitor(mExecutor, new HashSet<>()); - final Monitor.Callback callback = mock(Monitor.Callback.class); - - // Adds the same instance multiple times. - monitor.addCallback(callback); - monitor.addCallback(callback); - monitor.addCallback(callback); + public void removeCallback_noFailureOnDoubleRemove() { + final Condition condition = mock(Condition.class); + final Monitor monitor = new Monitor(mExecutor); + final Monitor.Callback callback = + mock(Monitor.Callback.class); + final Monitor.Subscription.Token token = monitor.addSubscription( + new Monitor.Subscription.Builder(callback).addCondition(condition).build() + ); + monitor.removeSubscription(token); + mExecutor.runAllReady(); + // Ensure second removal doesn't cause an exception. + monitor.removeSubscription(token); mExecutor.runAllReady(); - - // Callback should only be triggered once. - verify(callback, times(1)).onConditionsChanged(true); } @Test public void removeCallback_shouldNoLongerReceiveUpdate() { final Condition condition = mock(Condition.class); - final Monitor monitor = new Monitor(mExecutor, new HashSet<>(Arrays.asList(condition))); + final Monitor monitor = new Monitor(mExecutor); final Monitor.Callback callback = mock(Monitor.Callback.class); - monitor.addCallback(callback); - monitor.removeCallback(callback); + final Monitor.Subscription.Token token = monitor.addSubscription( + new Monitor.Subscription.Builder(callback).addCondition(condition).build() + ); + monitor.removeSubscription(token); mExecutor.runAllReady(); clearInvocations(callback); final ArgumentCaptor<Condition.Callback> 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); - when(condition.isConditionMet()).thenReturn(false); - conditionCallback.onConditionChanged(condition); - mExecutor.runAllReady(); - verify(callback, never()).onConditionsChanged(false); + final Condition.Callback conditionCallback = conditionCallbackCaptor.getValue(); + verify(condition).removeCallback(conditionCallback); } @Test @@ -245,14 +256,16 @@ public class ConditionMonitorTest extends SysuiTestCase { mock(Monitor.Callback.class); final Monitor.Callback callback2 = mock(Monitor.Callback.class); - mConditionMonitor.addCallback(callback1); - mConditionMonitor.addCallback(callback2); + final Monitor.Subscription.Token subscription1 = + mConditionMonitor.addSubscription(getDefaultBuilder(callback1).build()); + final Monitor.Subscription.Token subscription2 = + mConditionMonitor.addSubscription(getDefaultBuilder(callback2).build()); - mConditionMonitor.removeCallback(callback1); + mConditionMonitor.removeSubscription(subscription1); mExecutor.runAllReady(); mConditions.forEach(condition -> verify(condition, never()).removeCallback(any())); - mConditionMonitor.removeCallback(callback2); + mConditionMonitor.removeSubscription(subscription2); mExecutor.runAllReady(); mConditions.forEach(condition -> verify(condition).removeCallback(any())); } @@ -261,7 +274,7 @@ public class ConditionMonitorTest extends SysuiTestCase { public void updateCallbacks_allConditionsMet_reportTrue() { final Monitor.Callback callback = mock(Monitor.Callback.class); - mConditionMonitor.addCallback(callback); + mConditionMonitor.addSubscription(getDefaultBuilder(callback).build()); clearInvocations(callback); mCondition1.fakeUpdateCondition(true); @@ -276,7 +289,7 @@ public class ConditionMonitorTest extends SysuiTestCase { public void updateCallbacks_oneConditionStoppedMeeting_reportFalse() { final Monitor.Callback callback = mock(Monitor.Callback.class); - mConditionMonitor.addCallback(callback); + mConditionMonitor.addSubscription(getDefaultBuilder(callback).build()); mCondition1.fakeUpdateCondition(true); mCondition2.fakeUpdateCondition(true); @@ -292,7 +305,7 @@ public class ConditionMonitorTest extends SysuiTestCase { public void updateCallbacks_shouldOnlyUpdateWhenValueChanges() { final Monitor.Callback callback = mock(Monitor.Callback.class); - mConditionMonitor.addCallback(callback); + mConditionMonitor.addSubscription(getDefaultBuilder(callback).build()); mExecutor.runAllReady(); verify(callback).onConditionsChanged(false); clearInvocations(callback); |