From daed6675fc8f837ff880ba2551221b488a4b8db2 Mon Sep 17 00:00:00 2001
From: Bryce Lee <brycelee@google.com>
Date: Fri, 10 Feb 2023 11:44:17 -0800
Subject: [PATCH] Always have ConditionalCoreStartable subscribe to monitor.

Currently, ConditionalCoreStartables bypass subscribing to the
condition monitor if no conditions are specified. However, a
monitor might have preconditions set on it that would be ignored
in this case. This changelist ensures the CoreStartable always
subscribes to that any preconditions from the monitor are
considered.

Test: atest ConditionalCoreStartableTest
Test: atest ConditionMonitorTest
Bug: 261420432
Change-Id: If4e14bca8c51a7568966950c429dae7c16fdf759
---
 .../condition/ConditionalCoreStartable.java   | 10 -------
 .../condition/ConditionMonitorTest.java       | 15 ++++++++++
 .../ConditionalCoreStartableTest.java         | 30 +++++++++++++++++++
 3 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/packages/SystemUI/src/com/android/systemui/util/condition/ConditionalCoreStartable.java b/packages/SystemUI/src/com/android/systemui/util/condition/ConditionalCoreStartable.java
index b41bca0d77e1e..8d32a4833471e 100644
--- a/packages/SystemUI/src/com/android/systemui/util/condition/ConditionalCoreStartable.java
+++ b/packages/SystemUI/src/com/android/systemui/util/condition/ConditionalCoreStartable.java
@@ -43,11 +43,6 @@ public abstract class ConditionalCoreStartable implements CoreStartable {
 
     @Override
     public final void start() {
-        if (mConditionSet == null || mConditionSet.isEmpty()) {
-            onStart();
-            return;
-        }
-
         mStartToken = mMonitor.addSubscription(
                 new Monitor.Subscription.Builder(allConditionsMet -> {
                     if (allConditionsMet) {
@@ -63,11 +58,6 @@ public abstract class ConditionalCoreStartable implements CoreStartable {
 
     @Override
     public final void onBootCompleted() {
-        if (mConditionSet == null || mConditionSet.isEmpty()) {
-            bootCompleted();
-            return;
-        }
-
         mBootCompletedToken = mMonitor.addSubscription(
                 new Monitor.Subscription.Builder(allConditionsMet -> {
                     if (allConditionsMet) {
diff --git a/packages/SystemUI/tests/src/com/android/systemui/shared/condition/ConditionMonitorTest.java b/packages/SystemUI/tests/src/com/android/systemui/shared/condition/ConditionMonitorTest.java
index 9eccbb6303abc..aa1636d8a0309 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/shared/condition/ConditionMonitorTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/shared/condition/ConditionMonitorTest.java
@@ -248,6 +248,21 @@ public class ConditionMonitorTest extends SysuiTestCase {
         verify(callback).onConditionsChanged(true);
     }
 
+    @Test
+    public void addCallback_preCondition_noConditions_reportAllConditionsMet() {
+        final Monitor
+                monitor = new Monitor(mExecutor, new HashSet<>(Arrays.asList(mCondition1)));
+        final Monitor.Callback callback = mock(
+                Monitor.Callback.class);
+
+        monitor.addSubscription(new Monitor.Subscription.Builder(callback).build());
+        mExecutor.runAllReady();
+        verify(callback, never()).onConditionsChanged(true);
+        mCondition1.fakeUpdateCondition(true);
+        mExecutor.runAllReady();
+        verify(callback).onConditionsChanged(true);
+    }
+
     @Test
     public void removeCallback_noFailureOnDoubleRemove() {
         final Condition condition = mock(
diff --git a/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionalCoreStartableTest.java b/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionalCoreStartableTest.java
index 5ef62c1e7e8dd..b367a603ec674 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionalCoreStartableTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionalCoreStartableTest.java
@@ -60,6 +60,11 @@ public class ConditionalCoreStartableTest extends SysuiTestCase {
             mCallback = callback;
         }
 
+        public FakeConditionalCoreStartable(Monitor monitor, Callback callback) {
+            super(monitor);
+            mCallback = callback;
+        }
+
         @Override
         protected void onStart() {
             mCallback.onStart();
@@ -122,6 +127,31 @@ public class ConditionalCoreStartableTest extends SysuiTestCase {
         verify(mMonitor).removeSubscription(mSubscriptionToken);
     }
 
+    @Test
+    public void testOnStartCallbackWithNoConditions() {
+        final CoreStartable coreStartable =
+                new FakeConditionalCoreStartable(mMonitor,
+                        mCallback);
+
+        when(mMonitor.addSubscription(any())).thenReturn(mSubscriptionToken);
+        coreStartable.start();
+
+        final ArgumentCaptor<Monitor.Subscription> subscriptionCaptor = ArgumentCaptor.forClass(
+                Monitor.Subscription.class);
+        verify(mMonitor).addSubscription(subscriptionCaptor.capture());
+
+        final Monitor.Subscription subscription = subscriptionCaptor.getValue();
+
+        assertThat(subscription.getConditions()).isEmpty();
+
+        verify(mCallback, never()).onStart();
+
+        subscription.getCallback().onConditionsChanged(true);
+
+        verify(mCallback).onStart();
+        verify(mMonitor).removeSubscription(mSubscriptionToken);
+    }
+
 
     /**
      * Verifies that {@link ConditionalCoreStartable#bootCompleted()} ()} is predicated on