Merge "Remove NEW_LIGHT_BAR_LOGIC flag" into udc-qpr-dev

packages/SystemUI/src/com/android/systemui/flags/Flags.kt

@@ -61,10 +61,6 @@ object Flags {
     // TODO(b/254512538): Tracking Bug
     val INSTANT_VOICE_REPLY = unreleasedFlag(111, "instant_voice_reply")
 
-    // TODO(b/279735475): Tracking Bug
-    @JvmField
-    val NEW_LIGHT_BAR_LOGIC = releasedFlag(279735475, "new_light_bar_logic")
-
     /**
      * This flag is server-controlled and should stay as [unreleasedFlag] since we never want to
      * enable it on release builds.

packages/SystemUI/src/com/android/systemui/statusbar/phone/LightBarController.java

@@ -38,8 +38,6 @@ import com.android.systemui.Dumpable;
 import com.android.systemui.R;
 import com.android.systemui.dagger.SysUISingleton;
 import com.android.systemui.dump.DumpManager;
-import com.android.systemui.flags.FeatureFlags;
-import com.android.systemui.flags.Flags;
 import com.android.systemui.navigationbar.NavigationModeController;
 import com.android.systemui.plugins.DarkIconDispatcher;
 import com.android.systemui.settings.DisplayTracker;
@@ -65,7 +63,6 @@ public class LightBarController implements BatteryController.BatteryStateChangeC
 
     private final SysuiDarkIconDispatcher mStatusBarIconController;
     private final BatteryController mBatteryController;
-    private final boolean mUseNewLightBarLogic;
     private BiometricUnlockController mBiometricUnlockController;
 
     private LightBarTransitionsController mNavigationBarController;
@@ -123,10 +120,8 @@ public class LightBarController implements BatteryController.BatteryStateChangeC
             DarkIconDispatcher darkIconDispatcher,
             BatteryController batteryController,
             NavigationModeController navModeController,
-            FeatureFlags featureFlags,
             DumpManager dumpManager,
             DisplayTracker displayTracker) {
-        mUseNewLightBarLogic = featureFlags.isEnabled(Flags.NEW_LIGHT_BAR_LOGIC);
         mDarkIconColor = ctx.getColor(R.color.dark_mode_icon_color_single_tone);
         mLightIconColor = ctx.getColor(R.color.light_mode_icon_color_single_tone);
         mStatusBarIconController = (SysuiDarkIconDispatcher) darkIconDispatcher;
@@ -188,7 +183,6 @@ public class LightBarController implements BatteryController.BatteryStateChangeC
             final boolean last = mNavigationLight;
             mHasLightNavigationBar = isLight(appearance, navigationBarMode,
                     APPEARANCE_LIGHT_NAVIGATION_BARS);
-            if (mUseNewLightBarLogic) {
             final boolean ignoreScrimForce = mDirectReplying && mNavbarColorManagedByIme;
             final boolean darkForScrim = mForceDarkForScrim && !ignoreScrimForce;
             final boolean lightForScrim = mForceLightForScrim && !ignoreScrimForce;
@@ -215,25 +209,6 @@ public class LightBarController implements BatteryController.BatteryStateChangeC
                         .toString();
                 if (DEBUG_LOGS) Log.d(TAG, mLastNavigationBarAppearanceChangedLog);
             }
-            } else {
-                mNavigationLight = mHasLightNavigationBar
-                        && (mDirectReplying && mNavbarColorManagedByIme || !mForceDarkForScrim)
-                        && !mQsCustomizing;
-                if (DEBUG_NAVBAR) {
-                    mLastNavigationBarAppearanceChangedLog = getLogStringBuilder()
-                            .append("onNavigationBarAppearanceChanged()")
-                            .append(" appearance=").append(appearance)
-                            .append(" nbModeChanged=").append(nbModeChanged)
-                            .append(" navigationBarMode=").append(navigationBarMode)
-                            .append(" navbarColorManagedByIme=").append(navbarColorManagedByIme)
-                            .append(" mHasLightNavigationBar=").append(mHasLightNavigationBar)
-                            .append(" mNavigationLight=").append(mNavigationLight)
-                            .append(" last=").append(last)
-                            .append(" timestamp=").append(System.currentTimeMillis())
-                            .toString();
-                    if (DEBUG_LOGS) Log.d(TAG, mLastNavigationBarAppearanceChangedLog);
-                }
-            }
             if (mNavigationLight != last) {
                 updateNavigation();
             }
@@ -311,7 +286,6 @@ public class LightBarController implements BatteryController.BatteryStateChangeC
 
     public void setScrimState(ScrimState scrimState, float scrimBehindAlpha,
             GradientColors scrimInFrontColor) {
-        if (mUseNewLightBarLogic) {
         boolean bouncerVisibleLast = mBouncerVisible;
         boolean forceDarkForScrimLast = mForceDarkForScrim;
         boolean forceLightForScrimLast = mForceLightForScrim;
@@ -346,32 +320,6 @@ public class LightBarController implements BatteryController.BatteryStateChangeC
                     .toString();
             if (DEBUG_LOGS) Log.d(TAG, mLastSetScrimStateLog);
         }
-        } else {
-            boolean forceDarkForScrimLast = mForceDarkForScrim;
-            // For BOUNCER/BOUNCER_SCRIMMED cases, we assume that alpha is always below threshold.
-            // This enables IMEs to control the navigation bar color.
-            // For other cases, scrim should be able to veto the light navigation bar.
-            // NOTE: this was also wrong for S and has been removed in the new logic.
-            mForceDarkForScrim = scrimState != ScrimState.BOUNCER
-                    && scrimState != ScrimState.BOUNCER_SCRIMMED
-                    && scrimBehindAlpha >= NAV_BAR_INVERSION_SCRIM_ALPHA_THRESHOLD
-                    && !scrimInFrontColor.supportsDarkText();
-            if (mHasLightNavigationBar && (mForceDarkForScrim != forceDarkForScrimLast)) {
-                reevaluate();
-            }
-            if (DEBUG_NAVBAR) {
-                mLastSetScrimStateLog = getLogStringBuilder()
-                        .append("setScrimState()")
-                        .append(" scrimState=").append(scrimState)
-                        .append(" scrimBehindAlpha=").append(scrimBehindAlpha)
-                        .append(" scrimInFrontColor=").append(scrimInFrontColor)
-                        .append(" mHasLightNavigationBar=").append(mHasLightNavigationBar)
-                        .append(" mForceDarkForScrim=").append(mForceDarkForScrim)
-                        .append(" timestamp=").append(System.currentTimeMillis())
-                        .toString();
-                if (DEBUG_LOGS) Log.d(TAG, mLastSetScrimStateLog);
-            }
-        }
     }
 
     @NonNull
@@ -498,7 +446,6 @@ public class LightBarController implements BatteryController.BatteryStateChangeC
         private final DarkIconDispatcher mDarkIconDispatcher;
         private final BatteryController mBatteryController;
         private final NavigationModeController mNavModeController;
-        private final FeatureFlags mFeatureFlags;
         private final DumpManager mDumpManager;
         private final DisplayTracker mDisplayTracker;
 
@@ -507,14 +454,12 @@ public class LightBarController implements BatteryController.BatteryStateChangeC
                 DarkIconDispatcher darkIconDispatcher,
                 BatteryController batteryController,
                 NavigationModeController navModeController,
-                FeatureFlags featureFlags,
                 DumpManager dumpManager,
                 DisplayTracker displayTracker) {
 
             mDarkIconDispatcher = darkIconDispatcher;
             mBatteryController = batteryController;
             mNavModeController = navModeController;
-            mFeatureFlags = featureFlags;
             mDumpManager = dumpManager;
             mDisplayTracker = displayTracker;
         }
@@ -522,7 +467,7 @@ public class LightBarController implements BatteryController.BatteryStateChangeC
         /** Create an {@link LightBarController} */
         public LightBarController create(Context context) {
             return new LightBarController(context, mDarkIconDispatcher, mBatteryController,
-                    mNavModeController, mFeatureFlags, mDumpManager, mDisplayTracker);
+                    mNavModeController, mDumpManager, mDisplayTracker);
         }
     }
 }

packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimController.java

@@ -56,8 +56,6 @@ import com.android.systemui.bouncer.shared.constants.KeyguardBouncerConstants;
 import com.android.systemui.dagger.SysUISingleton;
 import com.android.systemui.dagger.qualifiers.Main;
 import com.android.systemui.dock.DockManager;
-import com.android.systemui.flags.FeatureFlags;
-import com.android.systemui.flags.Flags;
 import com.android.systemui.keyguard.KeyguardUnlockAnimationController;
 import com.android.systemui.keyguard.domain.interactor.KeyguardTransitionInteractor;
 import com.android.systemui.keyguard.shared.model.ScrimAlpha;
@@ -249,8 +247,6 @@ public class ScrimController implements ViewTreeObserver.OnPreDrawListener, Dump
     private int mScrimsVisibility;
     private final TriConsumer<ScrimState, Float, GradientColors> mScrimStateListener;
     private final LargeScreenShadeInterpolator mLargeScreenShadeInterpolator;
-    private final FeatureFlags mFeatureFlags;
-    private final boolean mUseNewLightBarLogic;
     private Consumer<Integer> mScrimVisibleListener;
     private boolean mBlankScreen;
     private boolean mScreenBlankingCallbackCalled;
@@ -303,12 +299,9 @@ public class ScrimController implements ViewTreeObserver.OnPreDrawListener, Dump
             PrimaryBouncerToGoneTransitionViewModel primaryBouncerToGoneTransitionViewModel,
             KeyguardTransitionInteractor keyguardTransitionInteractor,
             @Main CoroutineDispatcher mainDispatcher,
-            LargeScreenShadeInterpolator largeScreenShadeInterpolator,
+            LargeScreenShadeInterpolator largeScreenShadeInterpolator) {
-            FeatureFlags featureFlags) {
         mScrimStateListener = lightBarController::setScrimState;
         mLargeScreenShadeInterpolator = largeScreenShadeInterpolator;
-        mFeatureFlags = featureFlags;
-        mUseNewLightBarLogic = featureFlags.isEnabled(Flags.NEW_LIGHT_BAR_LOGIC);
         mDefaultScrimAlpha = BUSY_SCRIM_ALPHA;
 
         mKeyguardStateController = keyguardStateController;
@@ -1153,13 +1146,7 @@ public class ScrimController implements ViewTreeObserver.OnPreDrawListener, Dump
         if (mClipsQsScrim && mQsBottomVisible) {
             alpha = mNotificationsAlpha;
         }
-        if (mUseNewLightBarLogic) {
         mScrimStateListener.accept(mState, alpha, mColors);
-        } else {
-            // NOTE: This wasn't wrong, but it implied that each scrim might have different colors,
-            //  when in fact they all share the same GradientColors instance, which we own.
-            mScrimStateListener.accept(mState, alpha, mScrimInFront.getColors());
-        }
     }
 
     private void dispatchScrimsVisible() {
@@ -1483,15 +1470,8 @@ public class ScrimController implements ViewTreeObserver.OnPreDrawListener, Dump
         int accent = Utils.getColorAccent(mScrimBehind.getContext()).getDefaultColor();
         mColors.setMainColor(background);
         mColors.setSecondaryColor(accent);
-        if (mUseNewLightBarLogic) {
         final boolean isBackgroundLight = !ContrastColorUtil.isColorDark(background);
         mColors.setSupportsDarkText(isBackgroundLight);
-        } else {
-            // NOTE: This was totally backward, but LightBarController was flipping it back.
-            // There may be other consumers of this which would struggle though
-            mColors.setSupportsDarkText(
-                    ColorUtils.calculateContrast(mColors.getMainColor(), Color.WHITE) > 4.5);
-        }
 
         int surface = Utils.getColorAttr(mScrimBehind.getContext(),
                 com.android.internal.R.attr.materialColorSurface).getDefaultColor();

packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LightBarControllerTest.java

@@ -22,7 +22,6 @@ import static com.android.systemui.statusbar.phone.BarTransitions.MODE_TRANSPARE
 
 import static junit.framework.Assert.assertTrue;
 
-import static org.junit.Assume.assumeTrue;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
@@ -45,8 +44,6 @@ import com.android.internal.util.ContrastColorUtil;
 import com.android.internal.view.AppearanceRegion;
 import com.android.systemui.SysuiTestCase;
 import com.android.systemui.dump.DumpManager;
-import com.android.systemui.flags.FakeFeatureFlags;
-import com.android.systemui.flags.Flags;
 import com.android.systemui.navigationbar.NavigationModeController;
 import com.android.systemui.settings.FakeDisplayTracker;
 import com.android.systemui.statusbar.policy.BatteryController;
@@ -65,20 +62,13 @@ public class LightBarControllerTest extends SysuiTestCase {
 
     private static final GradientColors COLORS_LIGHT = makeColors(Color.WHITE);
     private static final GradientColors COLORS_DARK = makeColors(Color.BLACK);
-    private final FakeFeatureFlags mFeatureFlags = new FakeFeatureFlags();
     private LightBarTransitionsController mLightBarTransitionsController;
     private LightBarTransitionsController mNavBarController;
     private SysuiDarkIconDispatcher mStatusBarIconController;
     private LightBarController mLightBarController;
 
-    /** Allow testing with NEW_LIGHT_BAR_LOGIC flag in different states */
-    protected boolean testNewLightBarLogic() {
-        return false;
-    }
-
     @Before
     public void setup() {
-        mFeatureFlags.set(Flags.NEW_LIGHT_BAR_LOGIC, testNewLightBarLogic());
         mStatusBarIconController = mock(SysuiDarkIconDispatcher.class);
         mNavBarController = mock(LightBarTransitionsController.class);
         when(mNavBarController.supportsIconTintForNavMode(anyInt())).thenReturn(true);
@@ -90,7 +80,6 @@ public class LightBarControllerTest extends SysuiTestCase {
                 mStatusBarIconController,
                 mock(BatteryController.class),
                 mock(NavigationModeController.class),
-                mFeatureFlags,
                 mock(DumpManager.class),
                 new FakeDisplayTracker(mContext));
     }
@@ -211,8 +200,6 @@ public class LightBarControllerTest extends SysuiTestCase {
 
     @Test
     public void validateNavBarChangesUpdateIcons() {
-        assumeTrue(testNewLightBarLogic());  // Only run in the new suite
-
         // On the launcher in dark mode buttons are light
         mLightBarController.setScrimState(ScrimState.UNLOCKED, 0f, COLORS_DARK);
         mLightBarController.onNavigationBarAppearanceChanged(
@@ -251,8 +238,6 @@ public class LightBarControllerTest extends SysuiTestCase {
 
     @Test
     public void navBarHasDarkIconsInLockedShade_lightMode() {
-        assumeTrue(testNewLightBarLogic());  // Only run in the new suite
-
         // On the locked shade QS in light mode buttons are light
         mLightBarController.setScrimState(ScrimState.SHADE_LOCKED, 1f, COLORS_LIGHT);
         mLightBarController.onNavigationBarAppearanceChanged(
@@ -287,8 +272,6 @@ public class LightBarControllerTest extends SysuiTestCase {
 
     @Test
     public void navBarHasLightIconsInLockedShade_darkMode() {
-        assumeTrue(testNewLightBarLogic());  // Only run in the new suite
-
         // On the locked shade QS in light mode buttons are light
         mLightBarController.setScrimState(ScrimState.SHADE_LOCKED, 1f, COLORS_DARK);
         mLightBarController.onNavigationBarAppearanceChanged(

packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LightBarControllerWithNewLogicTest.kt

@@ -1,28 +0,0 @@
-/*
- * 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.statusbar.phone
-
-import androidx.test.filters.SmallTest
-import com.android.systemui.flags.Flags.NEW_LIGHT_BAR_LOGIC
-
-/**
- * This file only needs to live as long as [NEW_LIGHT_BAR_LOGIC] does. When we delete that flag, we
- * can roll this back into the old test.
- */
-@SmallTest
-class LightBarControllerWithNewLogicTest : LightBarControllerTest() {
-    override fun testNewLightBarLogic(): Boolean = true
-}

packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ScrimControllerTest.java

@@ -64,7 +64,6 @@ import com.android.systemui.SysuiTestCase;
 import com.android.systemui.animation.ShadeInterpolation;
 import com.android.systemui.bouncer.shared.constants.KeyguardBouncerConstants;
 import com.android.systemui.dock.DockManager;
-import com.android.systemui.flags.FeatureFlags;
 import com.android.systemui.keyguard.KeyguardUnlockAnimationController;
 import com.android.systemui.keyguard.domain.interactor.KeyguardTransitionInteractor;
 import com.android.systemui.keyguard.shared.model.KeyguardState;
@@ -142,7 +141,6 @@ public class ScrimControllerTest extends SysuiTestCase {
     // TODO(b/204991468): Use a real PanelExpansionStateManager object once this bug is fixed. (The
     //   event-dispatch-on-registration pattern caused some of these unit tests to fail.)
     @Mock private StatusBarKeyguardViewManager mStatusBarKeyguardViewManager;
-    @Mock private FeatureFlags mFeatureFlags;
 
     private static class AnimatorListener implements Animator.AnimatorListener {
         private int mNumStarts;
@@ -280,8 +278,7 @@ public class ScrimControllerTest extends SysuiTestCase {
                 mPrimaryBouncerToGoneTransitionViewModel,
                 mKeyguardTransitionInteractor,
                 mMainDispatcher,
-                mLinearLargeScreenShadeInterpolator,
+                mLinearLargeScreenShadeInterpolator);
-                mFeatureFlags);
         mScrimController.setScrimVisibleListener(visible -> mScrimVisibility = visible);
         mScrimController.attachViews(mScrimBehind, mNotificationsScrim, mScrimInFront);
         mScrimController.setAnimatorListener(mAnimatorListener);
@@ -974,8 +971,7 @@ public class ScrimControllerTest extends SysuiTestCase {
                 mPrimaryBouncerToGoneTransitionViewModel,
                 mKeyguardTransitionInteractor,
                 mMainDispatcher,
-                mLinearLargeScreenShadeInterpolator,
+                mLinearLargeScreenShadeInterpolator);
-                mFeatureFlags);
         mScrimController.setScrimVisibleListener(visible -> mScrimVisibility = visible);
         mScrimController.attachViews(mScrimBehind, mNotificationsScrim, mScrimInFront);
         mScrimController.setAnimatorListener(mAnimatorListener);