Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2fd7a0b3 authored by Evan Laird's avatar Evan Laird
Browse files

[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
Change-Id: I0e68037126a526fcf3614bf25683af3fa4e5aa96
Merged-In: I0e68037126a526fcf3614bf25683af3fa4e5aa96
parent 430e9fba

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt

+90 −75
Original line number Diff line number Diff line
@@ -25,6 +25,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
@@ -62,7 +63,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.
@@ -75,6 +76,28 @@ object MobileIconBinder { 
        var isCollecting = false



        view.repeatWhenAttached {

            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()

                        }

                    }

                }

            }



            lifecycleScope.launch {

                repeatOnLifecycle(Lifecycle.State.STARTED) {

                    logger.logCollectionStarted(view, viewModel)

                    isCollecting = true
@@ -89,17 +112,6 @@ object MobileIconBinder { 
                        }

                    }



                launch {

                    viewModel.isVisible.collect { isVisible ->

                        viewModel.verboseLogger?.logBinderReceivedVisibility(

                            view,

                            viewModel.subscriptionId,

                            isVisible

                        )

                        view.isVisible = isVisible

                    }

                }



                    // Set the icon for the triangle

                    launch {

                        viewModel.icon.distinctUntilChanged().collect { icon ->
@@ -145,7 +157,9 @@ object MobileIconBinder { 
                    launch { viewModel.activityOutVisible.collect { activityOut.isVisible = it } }



                    launch {

                    viewModel.activityContainerVisible.collect { activityContainer.isVisible = it }

                        viewModel.activityContainerVisible.collect {

                            activityContainer.isVisible = it

                        }

                    }



                    // Set the tint
@@ -171,6 +185,7 @@ object MobileIconBinder { 
                    }

                }

            }

        }



        return object : ModernStatusBarViewBinding {

            override fun getShouldIconBeVisible(): Boolean {

packages/SystemUI/tests/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerTest.java

+10 −2
Original line number Diff line number Diff line
@@ -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;
@@ -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);

    }

packages/SystemUI/tests/src/com/android/systemui/util/kotlin/FlowProvider.kt

0 → 100644
+22 −0
Original line number Diff line number Diff line 
/*

 * 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)