From 514f25bbf459230f6b52647ba0dcb88ab4e51825 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabi=C3=A1n=20Kozynski?= Date: Thu, 22 Jun 2023 15:13:49 -0400 Subject: [PATCH] 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 --- .../com/android/systemui/qs/QSTileHost.java | 15 ++++++----- .../android/systemui/qs/dagger/QSModule.java | 14 +++++----- .../statusbar/phone/AutoTileManager.java | 16 +++++++++--- .../android/systemui/qs/QSTileHostTest.java | 17 ++++++++++-- .../statusbar/phone/AutoTileManagerTest.java | 26 ++++++++++++++----- 5 files changed, 62 insertions(+), 26 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java b/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java index d2568ac79105a..b5b476bc1ac83 100644 --- 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, 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, 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, P newValue = mContext.getResources().getString(R.string.quick_settings_tiles_retail_mode); } final List 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( diff --git a/packages/SystemUI/src/com/android/systemui/qs/dagger/QSModule.java b/packages/SystemUI/src/com/android/systemui/qs/dagger/QSModule.java index dffe7fd5f8183..03de3a02da138 100644 --- 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, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java index f6d53b3bc9b3b..5eafa9e970885 100644 --- 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) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java index d98bcee1e01ad..41c2746fcde84 100644 --- 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 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); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/AutoTileManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/AutoTileManagerTest.java index e680a4ec19d15..e4a2236211b1e 100644 --- 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);