Fix NightDisplayListener in AutoTileManager

Before this change, NightDisplayListener was injected once (on SystemUI start) and it was associated with the user upon creation. This had two problems: * The user didn't change when the actual user changed, meaning that it couldn't track changes in the state for a different user. * If the user associated with it was removed (and its Settings removed), then when the user changed it will try to reregister and fail. To fix this, inject a builder and always create a NightDisplayListener for the proper user. Also, when QS_PIPELINE_NEW_HOST is true and QS_PIPELINE_AUTO_ADD is false, the user changes were not propagated to AutoTileManager. This is fixed in this CL. Test: manual Test: atest AutoTileManager QSTileHost Fixes: 288403490 Change-Id: Iffb0d9b25ad36b599e9e4e4424985197435b4c8f
2023-06-22 15:13:49 -04:00
parent c268174cb0
commit 514f25bbf4
5 changed files with 62 additions and 26 deletions
--- a/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java
@@ -152,6 +152,7 @@ public class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, P
        mQsFactories.add(defaultFactory);
        pluginManager.addPluginListener(this, QSFactory.class, true);
        mUserTracker = userTracker;
+        mCurrentUser = userTracker.getUserId();
        mSecureSettings = secureSettings;
        mCustomTileStatePersister = customTileStatePersister;

@@ -272,6 +273,13 @@ public class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, P
        if (!TILES_SETTING.equals(key)) {
            return;
        }
+        int currentUser = mUserTracker.getUserId();
+        if (currentUser != mCurrentUser) {
+            mUserContext = mUserTracker.getUserContext();
+            if (mAutoTiles != null) {
+                mAutoTiles.changeUser(UserHandle.of(currentUser));
+            }
+        }
        // Do not process tiles if the flag is enabled.
        if (mFeatureFlags.isEnabled(Flags.QS_PIPELINE_NEW_HOST)) {
            return;
@@ -280,13 +288,6 @@ public class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, P
            newValue = mContext.getResources().getString(R.string.quick_settings_tiles_retail_mode);
        }
        final List<String> tileSpecs = loadTileSpecs(mContext, newValue);
-        int currentUser = mUserTracker.getUserId();
-        if (currentUser != mCurrentUser) {
-            mUserContext = mUserTracker.getUserContext();
-            if (mAutoTiles != null) {
-                mAutoTiles.changeUser(UserHandle.of(currentUser));
-            }
-        }
        if (tileSpecs.equals(mTileSpecs) && currentUser == mCurrentUser) return;
        Log.d(TAG, "Recreating tiles: " + tileSpecs);
        mTiles.entrySet().stream().filter(tile -> !tileSpecs.contains(tile.getKey())).forEach(
--- a/packages/SystemUI/src/com/android/systemui/qs/dagger/QSModule.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/dagger/QSModule.java
@@ -19,9 +19,9 @@ package com.android.systemui.qs.dagger;
 import static com.android.systemui.qs.dagger.QSFlagsModule.RBC_AVAILABLE;

 import android.content.Context;
-import android.hardware.display.NightDisplayListener;
 import android.os.Handler;

+import com.android.systemui.dagger.NightDisplayListenerModule;
 import com.android.systemui.dagger.SysUISingleton;
 import com.android.systemui.dagger.qualifiers.Background;
 import com.android.systemui.media.dagger.MediaModule;
@@ -41,14 +41,14 @@ import com.android.systemui.statusbar.policy.SafetyController;
 import com.android.systemui.statusbar.policy.WalletController;
 import com.android.systemui.util.settings.SecureSettings;

-import dagger.Module;
-import dagger.Provides;
-import dagger.multibindings.Multibinds;
-
 import java.util.Map;

 import javax.inject.Named;

+import dagger.Module;
+import dagger.Provides;
+import dagger.multibindings.Multibinds;
+
 /**
 * Module for QS dependencies
 */
@@ -79,7 +79,7 @@ public interface QSModule {
            HotspotController hotspotController,
            DataSaverController dataSaverController,
            ManagedProfileController managedProfileController,
-            NightDisplayListener nightDisplayListener,
+            NightDisplayListenerModule.Builder nightDisplayListenerBuilder,
            CastController castController,
            ReduceBrightColorsController reduceBrightColorsController,
            DeviceControlsController deviceControlsController,
@@ -95,7 +95,7 @@ public interface QSModule {
                hotspotController,
                dataSaverController,
                managedProfileController,
-                nightDisplayListener,
+                nightDisplayListenerBuilder,
                castController,
                reduceBrightColorsController,
                deviceControlsController,
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java
@@ -20,6 +20,7 @@ import android.annotation.Nullable;
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.res.Resources;
+import android.database.ContentObserver;
 import android.hardware.display.ColorDisplayManager;
 import android.hardware.display.NightDisplayListener;
 import android.os.Handler;
@@ -28,6 +29,7 @@ import android.util.Log;

 import com.android.internal.annotations.VisibleForTesting;
 import com.android.systemui.R;
+import com.android.systemui.dagger.NightDisplayListenerModule;
 import com.android.systemui.dagger.qualifiers.Background;
 import com.android.systemui.plugins.qs.QSTile;
 import com.android.systemui.qs.AutoAddTracker;
@@ -82,7 +84,8 @@ public class AutoTileManager implements UserAwareController {
    private final HotspotController mHotspotController;
    private final DataSaverController mDataSaverController;
    private final ManagedProfileController mManagedProfileController;
-    private final NightDisplayListener mNightDisplayListener;
+    private final NightDisplayListenerModule.Builder mNightDisplayListenerBuilder;
+    private NightDisplayListener mNightDisplayListener;
    private final CastController mCastController;
    private final DeviceControlsController mDeviceControlsController;
    private final WalletController mWalletController;
@@ -98,7 +101,7 @@ public class AutoTileManager implements UserAwareController {
            HotspotController hotspotController,
            DataSaverController dataSaverController,
            ManagedProfileController managedProfileController,
-            NightDisplayListener nightDisplayListener,
+            NightDisplayListenerModule.Builder nightDisplayListenerBuilder,
            CastController castController,
            ReduceBrightColorsController reduceBrightColorsController,
            DeviceControlsController deviceControlsController,
@@ -114,7 +117,7 @@ public class AutoTileManager implements UserAwareController {
        mHotspotController = hotspotController;
        mDataSaverController = dataSaverController;
        mManagedProfileController = managedProfileController;
-        mNightDisplayListener = nightDisplayListener;
+        mNightDisplayListenerBuilder = nightDisplayListenerBuilder;
        mCastController = castController;
        mReduceBrightColorsController = reduceBrightColorsController;
        mIsReduceBrightColorsAvailable = isReduceBrightColorsAvailable;
@@ -157,6 +160,10 @@ public class AutoTileManager implements UserAwareController {
            mDataSaverController.addCallback(mDataSaverListener);
        }
        mManagedProfileController.addCallback(mProfileCallback);
+
+        mNightDisplayListener = mNightDisplayListenerBuilder
+                .setUser(mCurrentUser.getIdentifier())
+                .build();
        if (!mAutoTracker.isAdded(NIGHT)
                && ColorDisplayManager.isNightDisplayAvailable(mContext)) {
            mNightDisplayListener.setCallback(mNightDisplayCallback);
@@ -193,7 +200,8 @@ public class AutoTileManager implements UserAwareController {
        mHotspotController.removeCallback(mHotspotCallback);
        mDataSaverController.removeCallback(mDataSaverListener);
        mManagedProfileController.removeCallback(mProfileCallback);
-        if (ColorDisplayManager.isNightDisplayAvailable(mContext)) {
+        if (ColorDisplayManager.isNightDisplayAvailable(mContext)
+                && mNightDisplayListener != null) {
            mNightDisplayListener.setCallback(null);
        }
        if (mIsReduceBrightColorsAvailable) {
--- a/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java
@@ -25,6 +25,7 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -107,7 +108,7 @@ public class QSTileHostTest extends SysuiTestCase {
    @Mock
    private TunerService mTunerService;
    @Mock
-    private Provider<AutoTileManager> mAutoTiles;
+    private AutoTileManager mAutoTiles;
    @Mock
    private ShadeController mShadeController;
    @Mock
@@ -160,9 +161,10 @@ public class QSTileHostTest extends SysuiTestCase {
        mSecureSettings = new FakeSettings();
        saveSetting("");
        mQSTileHost = new TestQSTileHost(mContext, mDefaultFactory, mMainExecutor,
-                mPluginManager, mTunerService, mAutoTiles, mShadeController,
+                mPluginManager, mTunerService, () -> mAutoTiles, mShadeController,
                mQSLogger, mUserTracker, mSecureSettings, mCustomTileStatePersister,
                mTileLifecycleManagerFactory, mUserFileManager, mFeatureFlags);
+        mMainExecutor.runAllReady();

        mSecureSettings.registerContentObserverForUser(SETTING, new ContentObserver(null) {
            @Override
@@ -672,6 +674,17 @@ public class QSTileHostTest extends SysuiTestCase {
        assertEquals(CUSTOM_TILE.getClassName(), proto.tiles[1].getComponentName().className);
    }

+    @Test
+    public void testUserChange_flagOn_autoTileManagerNotified() {
+        mFeatureFlags.set(Flags.QS_PIPELINE_NEW_HOST, true);
+        int currentUser = mUserTracker.getUserId();
+        clearInvocations(mAutoTiles);
+        when(mUserTracker.getUserId()).thenReturn(currentUser + 1);
+
+        mQSTileHost.onTuningChanged(SETTING, "a,b");
+        verify(mAutoTiles).changeUser(UserHandle.of(currentUser + 1));
+    }
+
    private SharedPreferences getSharedPreferencesForUser(int user) {
        return mUserFileManager.getSharedPreferences(QSTileHost.TILES, 0, user);
    }
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/AutoTileManagerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/AutoTileManagerTest.java
@@ -34,6 +34,7 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;

 import android.content.ComponentName;
@@ -51,6 +52,7 @@ import androidx.test.filters.SmallTest;

 import com.android.systemui.R;
 import com.android.systemui.SysuiTestCase;
+import com.android.systemui.dagger.NightDisplayListenerModule;
 import com.android.systemui.plugins.qs.QSTile;
 import com.android.systemui.qs.AutoAddTracker;
 import com.android.systemui.qs.QSHost;
@@ -111,6 +113,8 @@ public class AutoTileManagerTest extends SysuiTestCase {
    @Mock private DataSaverController mDataSaverController;
    @Mock private ManagedProfileController mManagedProfileController;
    @Mock private NightDisplayListener mNightDisplayListener;
+    @Mock(answer = Answers.RETURNS_SELF)
+    private NightDisplayListenerModule.Builder mNightDisplayListenerBuilder;
    @Mock private ReduceBrightColorsController mReduceBrightColorsController;
    @Mock private DeviceControlsController mDeviceControlsController;
    @Mock private WalletController mWalletController;
@@ -151,6 +155,7 @@ public class AutoTileManagerTest extends SysuiTestCase {
                .thenReturn(TEST_CUSTOM_SAFETY_PKG);
        Context context = Mockito.spy(mContext);
        when(context.getPackageManager()).thenReturn(mPackageManager);
+        when(mNightDisplayListenerBuilder.build()).thenReturn(mNightDisplayListener);

        mAutoTileManager = createAutoTileManager(context);
        mAutoTileManager.init();
@@ -167,7 +172,7 @@ public class AutoTileManagerTest extends SysuiTestCase {
            HotspotController hotspotController,
            DataSaverController dataSaverController,
            ManagedProfileController managedProfileController,
-            NightDisplayListener nightDisplayListener,
+            NightDisplayListenerModule.Builder nightDisplayListenerBuilder,
            CastController castController,
            ReduceBrightColorsController reduceBrightColorsController,
            DeviceControlsController deviceControlsController,
@@ -180,7 +185,7 @@ public class AutoTileManagerTest extends SysuiTestCase {
                hotspotController,
                dataSaverController,
                managedProfileController,
-                nightDisplayListener,
+                mNightDisplayListenerBuilder,
                castController,
                reduceBrightColorsController,
                deviceControlsController,
@@ -191,7 +196,7 @@ public class AutoTileManagerTest extends SysuiTestCase {

    private AutoTileManager createAutoTileManager(Context context) {
        return createAutoTileManager(context, mAutoAddTrackerBuilder, mHotspotController,
-                mDataSaverController, mManagedProfileController, mNightDisplayListener,
+                mDataSaverController, mManagedProfileController, mNightDisplayListenerBuilder,
                mCastController, mReduceBrightColorsController, mDeviceControlsController,
                mWalletController, mSafetyController, mIsReduceBrightColorsAvailable);
    }
@@ -204,7 +209,7 @@ public class AutoTileManagerTest extends SysuiTestCase {
        HotspotController hC = mock(HotspotController.class);
        DataSaverController dSC = mock(DataSaverController.class);
        ManagedProfileController mPC = mock(ManagedProfileController.class);
-        NightDisplayListener nDS = mock(NightDisplayListener.class);
+        NightDisplayListenerModule.Builder nDSB = mock(NightDisplayListenerModule.Builder.class);
        CastController cC = mock(CastController.class);
        ReduceBrightColorsController rBC = mock(ReduceBrightColorsController.class);
        DeviceControlsController dCC = mock(DeviceControlsController.class);
@@ -212,14 +217,14 @@ public class AutoTileManagerTest extends SysuiTestCase {
        SafetyController sC = mock(SafetyController.class);

        AutoTileManager manager =
-                createAutoTileManager(mock(Context.class), builder, hC, dSC, mPC, nDS, cC, rBC,
+                createAutoTileManager(mock(Context.class), builder, hC, dSC, mPC, nDSB, cC, rBC,
                        dCC, wC, sC, true);

        verify(tracker, never()).initialize();
        verify(hC, never()).addCallback(any());
        verify(dSC, never()).addCallback(any());
        verify(mPC, never()).addCallback(any());
-        verify(nDS, never()).setCallback(any());
+        verifyNoMoreInteractions(nDSB);
        verify(cC, never()).addCallback(any());
        verify(rBC, never()).addCallback(any());
        verify(dCC, never()).setCallback(any());
@@ -615,6 +620,15 @@ public class AutoTileManagerTest extends SysuiTestCase {
        createAutoTileManager(mContext).destroy();
    }

+    @Test
+    public void testUserChange_newNightDisplayListenerCreated() {
+        UserHandle newUser = UserHandle.of(1000);
+        mAutoTileManager.changeUser(newUser);
+        InOrder inOrder = inOrder(mNightDisplayListenerBuilder);
+        inOrder.verify(mNightDisplayListenerBuilder).setUser(newUser.getIdentifier());
+        inOrder.verify(mNightDisplayListenerBuilder).build();
+    }
+
    // Will only notify if it's listening
    private void changeValue(String key, int value) {
        mSecureSettings.putIntForUser(key, value, USER);