[StatusBar] Request layout on vis change, and move to CREATED

The flow that was being bound to the mobile icon group's visibility
lived in STARTED (this caused tests to pass). It also was experiencing a
race condition against the StatusIconContainer. In certain
circumstances, the view was being created and sent to have an icon state
of HIDDEN, despite having changed itself to be VISIBLE.

This is all emergent behavior from StatusIconContainer, which decides
which state to set on icons in its onMeasure(), but then applies that
state in onLayout().

To test: remove battery percent from status bar; ensure that _no other
icon_ is showing. This means disable Wi-Fi and make sure volume is set
to vibrate or on, since those states have no icon. Once you are in a
state where there is only battery and mobile showing, remove and
re-insert the SIM card to reproduce.

Test: ModernStatusBarMobileViewTest
Test: manually removing and inserting SIM card
Fixes: 291031862
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:53657d1ff8ba10647c38d8ad5fd0486202b2dedd)
Merged-In: I0e68037126a526fcf3614bf25683af3fa4e5aa96
Change-Id: I0e68037126a526fcf3614bf25683af3fa4e5aa96

3 files changed, 130 insertions, 85 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt
index 4b2fb4373957..249ca35b610a 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt
@@ -26,6 +26,7 @@ import android.widget.ImageView
 import android.widget.Space
 import androidx.core.view.isVisible
 import androidx.lifecycle.Lifecycle
+import androidx.lifecycle.lifecycleScope
 import androidx.lifecycle.repeatOnLifecycle
 import com.android.settingslib.graph.SignalDrawable
 import com.android.systemui.R
@@ -64,7 +65,7 @@ object MobileIconBinder {
         val roamingSpace = view.requireViewById<Space>(R.id.mobile_roaming_space)
         val dotView = view.requireViewById<StatusBarIconView>(R.id.status_bar_dot)
 
-        view.isVisible = true
+        view.isVisible = viewModel.isVisible.value
         iconView.isVisible = true
 
         // TODO(b/238425913): We should log this visibility state.
@@ -77,108 +78,122 @@ object MobileIconBinder {
         var isCollecting = false
 
         view.repeatWhenAttached {
-            repeatOnLifecycle(Lifecycle.State.STARTED) {
-                logger.logCollectionStarted(view, viewModel)
-                isCollecting = true
-
-                launch {
-                    visibilityState.collect { state ->
-                        when (state) {
-                            STATE_ICON -> {
-                                mobileGroupView.visibility = VISIBLE
-                                dotView.visibility = GONE
-                            }
-                            STATE_DOT -> {
-                                mobileGroupView.visibility = INVISIBLE
-                                dotView.visibility = VISIBLE
-                            }
-                            STATE_HIDDEN -> {
-                                mobileGroupView.visibility = INVISIBLE
-                                dotView.visibility = INVISIBLE
-                            }
+            lifecycleScope.launch {
+                repeatOnLifecycle(Lifecycle.State.CREATED) {
+                    // isVisible controls the visibility state of the outer group, and thus it needs
+                    // to run in the CREATED lifecycle so it can continue to watch while invisible
+                    // See (b/291031862) for details
+                    launch {
+                        viewModel.isVisible.collect { isVisible ->
+                            viewModel.verboseLogger?.logBinderReceivedVisibility(
+                                view,
+                                viewModel.subscriptionId,
+                                isVisible
+                            )
+                            view.isVisible = isVisible
+                            // [StatusIconContainer] can get out of sync sometimes. Make sure to
+                            // request another layout when this changes.
+                            view.requestLayout()
                         }
                     }
                 }
+            }
 
-                launch {
-                    viewModel.isVisible.collect { isVisible ->
-                        viewModel.verboseLogger?.logBinderReceivedVisibility(
-                            view,
-                            viewModel.subscriptionId,
-                            isVisible
-                        )
-                        view.isVisible = isVisible
+            lifecycleScope.launch {
+                repeatOnLifecycle(Lifecycle.State.STARTED) {
+                    logger.logCollectionStarted(view, viewModel)
+                    isCollecting = true
+
+                    launch {
+                        visibilityState.collect { state ->
+                            when (state) {
+                                STATE_ICON -> {
+                                    mobileGroupView.visibility = VISIBLE
+                                    dotView.visibility = GONE
+                                }
+                                STATE_DOT -> {
+                                    mobileGroupView.visibility = INVISIBLE
+                                    dotView.visibility = VISIBLE
+                                }
+                                STATE_HIDDEN -> {
+                                    mobileGroupView.visibility = INVISIBLE
+                                    dotView.visibility = INVISIBLE
+                                }
+                            }
+                        }
                     }
-                }
 
-                // Set the icon for the triangle
-                launch {
-                    viewModel.icon.distinctUntilChanged().collect { icon ->
-                        viewModel.verboseLogger?.logBinderReceivedSignalIcon(
-                            view,
-                            viewModel.subscriptionId,
-                            icon,
-                        )
-                        mobileDrawable.level = icon.toSignalDrawableState()
+                    // Set the icon for the triangle
+                    launch {
+                        viewModel.icon.distinctUntilChanged().collect { icon ->
+                            viewModel.verboseLogger?.logBinderReceivedSignalIcon(
+                                view,
+                                viewModel.subscriptionId,
+                                icon,
+                            )
+                            mobileDrawable.level = icon.toSignalDrawableState()
+                        }
                     }
-                }
 
-                launch {
-                    viewModel.contentDescription.distinctUntilChanged().collect {
-                        ContentDescriptionViewBinder.bind(it, view)
+                    launch {
+                        viewModel.contentDescription.distinctUntilChanged().collect {
+                            ContentDescriptionViewBinder.bind(it, view)
+                        }
                     }
-                }
 
-                // Set the network type icon
-                launch {
-                    viewModel.networkTypeIcon.distinctUntilChanged().collect { dataTypeId ->
-                        viewModel.verboseLogger?.logBinderReceivedNetworkTypeIcon(
-                            view,
-                            viewModel.subscriptionId,
-                            dataTypeId,
-                        )
-                        dataTypeId?.let { IconViewBinder.bind(dataTypeId, networkTypeView) }
-                        networkTypeView.visibility = if (dataTypeId != null) VISIBLE else GONE
+                    // Set the network type icon
+                    launch {
+                        viewModel.networkTypeIcon.distinctUntilChanged().collect { dataTypeId ->
+                            viewModel.verboseLogger?.logBinderReceivedNetworkTypeIcon(
+                                view,
+                                viewModel.subscriptionId,
+                                dataTypeId,
+                            )
+                            dataTypeId?.let { IconViewBinder.bind(dataTypeId, networkTypeView) }
+                            networkTypeView.visibility = if (dataTypeId != null) VISIBLE else GONE
+                        }
                     }
-                }
 
-                // Set the roaming indicator
-                launch {
-                    viewModel.roaming.distinctUntilChanged().collect { isRoaming ->
-                        roamingView.isVisible = isRoaming
-                        roamingSpace.isVisible = isRoaming
+                    // Set the roaming indicator
+                    launch {
+                        viewModel.roaming.distinctUntilChanged().collect { isRoaming ->
+                            roamingView.isVisible = isRoaming
+                            roamingSpace.isVisible = isRoaming
+                        }
                     }
-                }
 
-                // Set the activity indicators
-                launch { viewModel.activityInVisible.collect { activityIn.isVisible = it } }
+                    // Set the activity indicators
+                    launch { viewModel.activityInVisible.collect { activityIn.isVisible = it } }
 
-                launch { viewModel.activityOutVisible.collect { activityOut.isVisible = it } }
+                    launch { viewModel.activityOutVisible.collect { activityOut.isVisible = it } }
 
-                launch {
-                    viewModel.activityContainerVisible.collect { activityContainer.isVisible = it }
-                }
+                    launch {
+                        viewModel.activityContainerVisible.collect {
+                            activityContainer.isVisible = it
+                        }
+                    }
 
-                // Set the tint
-                launch {
-                    iconTint.collect { tint ->
-                        val tintList = ColorStateList.valueOf(tint)
-                        iconView.imageTintList = tintList
-                        networkTypeView.imageTintList = tintList
-                        roamingView.imageTintList = tintList
-                        activityIn.imageTintList = tintList
-                        activityOut.imageTintList = tintList
-                        dotView.setDecorColor(tint)
+                    // Set the tint
+                    launch {
+                        iconTint.collect { tint ->
+                            val tintList = ColorStateList.valueOf(tint)
+                            iconView.imageTintList = tintList
+                            networkTypeView.imageTintList = tintList
+                            roamingView.imageTintList = tintList
+                            activityIn.imageTintList = tintList
+                            activityOut.imageTintList = tintList
+                            dotView.setDecorColor(tint)
+                        }
                     }
-                }
 
-                launch { decorTint.collect { tint -> dotView.setDecorColor(tint) } }
+                    launch { decorTint.collect { tint -> dotView.setDecorColor(tint) } }
 
-                try {
-                    awaitCancellation()
-                } finally {
-                    isCollecting = false
-                    logger.logCollectionStopped(view, viewModel)
+                    try {
+                        awaitCancellation()
+                    } finally {
+                        isCollecting = false
+                        logger.logCollectionStopped(view, viewModel)
+                    }
                 }
             }
         }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerTest.java
index 5fa6b3a15d35..e7056c7b0b9d 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerTest.java
@@ -58,6 +58,7 @@ import com.android.systemui.statusbar.pipeline.mobile.ui.MobileViewLogger;
 import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.MobileIconsViewModel;
 import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.ShadeCarrierGroupMobileIconViewModel;
 import com.android.systemui.util.CarrierConfigTracker;
+import com.android.systemui.util.kotlin.FlowProviderKt;
 import com.android.systemui.utils.leaks.LeakCheckedTest;
 import com.android.systemui.utils.os.FakeHandler;
 
@@ -72,6 +73,8 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
+import kotlinx.coroutines.flow.MutableStateFlow;
+
 @RunWith(AndroidTestingRunner.class)
 @TestableLooper.RunWithLooper
 @SmallTest
@@ -105,7 +108,8 @@ public class ShadeCarrierGroupControllerTest extends LeakCheckedTest {
     private ShadeCarrier mShadeCarrier3;
     private TestableLooper mTestableLooper;
     @Mock
-    private ShadeCarrierGroupController.OnSingleCarrierChangedListener mOnSingleCarrierChangedListener;
+    private ShadeCarrierGroupController.OnSingleCarrierChangedListener
+            mOnSingleCarrierChangedListener;
     @Mock
     private MobileUiAdapter mMobileUiAdapter;
     @Mock
@@ -119,6 +123,9 @@ public class ShadeCarrierGroupControllerTest extends LeakCheckedTest {
     @Mock
     private StatusBarPipelineFlags mStatusBarPipelineFlags;
 
+    private final MutableStateFlow<Boolean> mIsVisibleFlow =
+            FlowProviderKt.getMutableStateFlow(true);
+
     private FakeSlotIndexResolver mSlotIndexResolver;
     private ClickListenerTextView mNoCarrierTextView;
 
@@ -170,7 +177,7 @@ public class ShadeCarrierGroupControllerTest extends LeakCheckedTest {
                 mMobileUiAdapter,
                 mMobileContextProvider,
                 mStatusBarPipelineFlags
-            )
+        )
                 .setShadeCarrierGroup(mShadeCarrierGroup)
                 .build();
 
@@ -181,6 +188,7 @@ public class ShadeCarrierGroupControllerTest extends LeakCheckedTest {
         when(mStatusBarPipelineFlags.useNewShadeCarrierGroupMobileIcons()).thenReturn(true);
         when(mMobileContextProvider.getMobileContextForSub(anyInt(), any())).thenReturn(mContext);
         when(mMobileIconsViewModel.getLogger()).thenReturn(mMobileViewLogger);
+        when(mShadeCarrierGroupMobileIconViewModel.isVisible()).thenReturn(mIsVisibleFlow);
         when(mMobileIconsViewModel.viewModelForSub(anyInt(), any()))
                 .thenReturn(mShadeCarrierGroupMobileIconViewModel);
     }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/util/kotlin/FlowProvider.kt b/packages/SystemUI/tests/src/com/android/systemui/util/kotlin/FlowProvider.kt
new file mode 100644
index 000000000000..274acc9a8c23
--- /dev/null
+++ b/packages/SystemUI/tests/src/com/android/systemui/util/kotlin/FlowProvider.kt
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2023 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.kotlin
+
+import kotlinx.coroutines.flow.MutableStateFlow
+
+/** Wrapper for flow constructors that can be retrieved from java tests */
+fun <T> getMutableStateFlow(value: T): MutableStateFlow<T> = MutableStateFlow(value)