From c0143c7ad60921c0800ec7470b815d9752c4d3df Mon Sep 17 00:00:00 2001 From: Shan Huang Date: Wed, 25 Aug 2021 23:39:59 +0000 Subject: [PATCH] Fix race condition when setting boot color sysproprs. Bug: 197690550 Test: ThemeOverlayApplierTest, ThemeOverlayControllerTest Change-Id: I514bf94c1e7c26072f729cb8bf5295d9574b6f02 --- .../systemui/dagger/DependencyProvider.java | 6 ++- .../systemui/theme/ThemeOverlayApplier.java | 18 ++++--- .../theme/ThemeOverlayController.java | 6 +-- .../theme/ThemeOverlayApplierTest.java | 22 ++++---- .../theme/ThemeOverlayControllerTest.java | 50 +++++++++---------- 5 files changed, 57 insertions(+), 45 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/dagger/DependencyProvider.java b/packages/SystemUI/src/com/android/systemui/dagger/DependencyProvider.java index c97a30e6e13e5..79ee6a8e88309 100644 --- a/packages/SystemUI/src/com/android/systemui/dagger/DependencyProvider.java +++ b/packages/SystemUI/src/com/android/systemui/dagger/DependencyProvider.java @@ -198,9 +198,11 @@ public class DependencyProvider { @SysUISingleton @Provides static ThemeOverlayApplier provideThemeOverlayManager(Context context, - @Background Executor bgExecutor, OverlayManager overlayManager, + @Background Executor bgExecutor, + @Main Executor mainExecutor, + OverlayManager overlayManager, DumpManager dumpManager) { - return new ThemeOverlayApplier(overlayManager, bgExecutor, + return new ThemeOverlayApplier(overlayManager, bgExecutor, mainExecutor, context.getString(R.string.launcher_overlayable_package), context.getString(R.string.themepicker_overlayable_package), dumpManager); } diff --git a/packages/SystemUI/src/com/android/systemui/theme/ThemeOverlayApplier.java b/packages/SystemUI/src/com/android/systemui/theme/ThemeOverlayApplier.java index c3b4fbe9a13dd..fe0b970560240 100644 --- a/packages/SystemUI/src/com/android/systemui/theme/ThemeOverlayApplier.java +++ b/packages/SystemUI/src/com/android/systemui/theme/ThemeOverlayApplier.java @@ -132,14 +132,18 @@ public class ThemeOverlayApplier implements Dumpable { /* Target package for each overlay category. */ private final Map mCategoryToTargetPackage = new ArrayMap<>(); private final OverlayManager mOverlayManager; - private final Executor mExecutor; + private final Executor mBgExecutor; + private final Executor mMainExecutor; private final String mLauncherPackage; private final String mThemePickerPackage; - public ThemeOverlayApplier(OverlayManager overlayManager, Executor executor, + public ThemeOverlayApplier(OverlayManager overlayManager, + Executor bgExecutor, + Executor mainExecutor, String launcherPackage, String themePickerPackage, DumpManager dumpManager) { mOverlayManager = overlayManager; - mExecutor = executor; + mBgExecutor = bgExecutor; + mMainExecutor = mainExecutor; mLauncherPackage = launcherPackage; mThemePickerPackage = themePickerPackage; mTargetPackageToCategories.put(ANDROID_PACKAGE, Sets.newHashSet( @@ -170,12 +174,13 @@ public class ThemeOverlayApplier implements Dumpable { * Apply the set of overlay packages to the set of {@code UserHandle}s provided. Overlays that * affect sysui will also be applied to the system user. */ - void applyCurrentUserOverlays( + public void applyCurrentUserOverlays( Map categoryToPackage, FabricatedOverlay[] pendingCreation, int currentUser, - Set managedProfiles) { - mExecutor.execute(() -> { + Set managedProfiles, + Runnable onOverlaysApplied) { + mBgExecutor.execute(() -> { // Disable all overlays that have not been specified in the user setting. final Set overlayCategoriesToDisable = new HashSet<>(THEME_CATEGORIES); @@ -221,6 +226,7 @@ public class ThemeOverlayApplier implements Dumpable { try { mOverlayManager.commit(transaction.build()); + mMainExecutor.execute(onOverlaysApplied); } catch (SecurityException | IllegalStateException e) { Log.e(TAG, "setEnabled failed", e); } diff --git a/packages/SystemUI/src/com/android/systemui/theme/ThemeOverlayController.java b/packages/SystemUI/src/com/android/systemui/theme/ThemeOverlayController.java index 6af49ff796688..c3327dfc6d4ed 100644 --- a/packages/SystemUI/src/com/android/systemui/theme/ThemeOverlayController.java +++ b/packages/SystemUI/src/com/android/systemui/theme/ThemeOverlayController.java @@ -521,16 +521,16 @@ public class ThemeOverlayController extends SystemUI implements Dumpable { .map(key -> key + " -> " + categoryToPackage.get(key)).collect( Collectors.joining(", "))); } + Runnable overlaysAppliedRunnable = () -> onOverlaysApplied(); if (mNeedsOverlayCreation) { mNeedsOverlayCreation = false; mThemeManager.applyCurrentUserOverlays(categoryToPackage, new FabricatedOverlay[] { mSecondaryOverlay, mNeutralOverlay - }, currentUser, managedProfiles); + }, currentUser, managedProfiles, overlaysAppliedRunnable); } else { mThemeManager.applyCurrentUserOverlays(categoryToPackage, null, currentUser, - managedProfiles); + managedProfiles, overlaysAppliedRunnable); } - onOverlaysApplied(); } protected void onOverlaysApplied() { diff --git a/packages/SystemUI/tests/src/com/android/systemui/theme/ThemeOverlayApplierTest.java b/packages/SystemUI/tests/src/com/android/systemui/theme/ThemeOverlayApplierTest.java index 9c47f19b20c8a..e6dc4db34f91e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/theme/ThemeOverlayApplierTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/theme/ThemeOverlayApplierTest.java @@ -96,6 +96,8 @@ public class ThemeOverlayApplierTest extends SysuiTestCase { DumpManager mDumpManager; @Mock OverlayManagerTransaction.Builder mTransactionBuilder; + @Mock + Runnable mOnOverlaysApplied; private ThemeOverlayApplier mManager; private boolean mGetOverlayInfoEnabled = true; @@ -103,7 +105,8 @@ public class ThemeOverlayApplierTest extends SysuiTestCase { @Before public void setup() throws Exception { MockitoAnnotations.initMocks(this); - mManager = new ThemeOverlayApplier(mOverlayManager, MoreExecutors.directExecutor(), + mManager = new ThemeOverlayApplier(mOverlayManager, + MoreExecutors.directExecutor(), MoreExecutors.directExecutor(), LAUNCHER_PACKAGE, THEMEPICKER_PACKAGE, mDumpManager) { @Override protected OverlayManagerTransaction.Builder getTransactionBuilder() { @@ -173,7 +176,7 @@ public class ThemeOverlayApplierTest extends SysuiTestCase { @Test public void allCategoriesSpecified_allEnabledExclusively() { mManager.applyCurrentUserOverlays(ALL_CATEGORIES_MAP, null, TEST_USER.getIdentifier(), - TEST_USER_HANDLES); + TEST_USER_HANDLES, mOnOverlaysApplied); verify(mOverlayManager).commit(any()); for (OverlayIdentifier overlayPackage : ALL_CATEGORIES_MAP.values()) { @@ -185,7 +188,7 @@ public class ThemeOverlayApplierTest extends SysuiTestCase { @Test public void allCategoriesSpecified_sysuiCategoriesAlsoAppliedToSysuiUser() { mManager.applyCurrentUserOverlays(ALL_CATEGORIES_MAP, null, TEST_USER.getIdentifier(), - TEST_USER_HANDLES); + TEST_USER_HANDLES, mOnOverlaysApplied); for (Map.Entry entry : ALL_CATEGORIES_MAP.entrySet()) { if (SYSTEM_USER_CATEGORIES.contains(entry.getKey())) { @@ -202,8 +205,9 @@ public class ThemeOverlayApplierTest extends SysuiTestCase { public void allCategoriesSpecified_enabledForAllUserHandles() { Set userHandles = Sets.newHashSet(TEST_USER_HANDLES); mManager.applyCurrentUserOverlays(ALL_CATEGORIES_MAP, null, TEST_USER.getIdentifier(), - userHandles); + userHandles, mOnOverlaysApplied); + verify(mOnOverlaysApplied).run(); for (OverlayIdentifier overlayPackage : ALL_CATEGORIES_MAP.values()) { verify(mTransactionBuilder).setEnabled(eq(overlayPackage), eq(true), eq(TEST_USER.getIdentifier())); @@ -219,7 +223,7 @@ public class ThemeOverlayApplierTest extends SysuiTestCase { Set userHandles = Sets.newHashSet(TEST_USER_HANDLES); mManager.applyCurrentUserOverlays(ALL_CATEGORIES_MAP, null, TEST_USER.getIdentifier(), - userHandles); + userHandles, mOnOverlaysApplied); for (OverlayIdentifier overlayPackage : ALL_CATEGORIES_MAP.values()) { verify(mTransactionBuilder, never()).setEnabled(eq(overlayPackage), eq(true), @@ -233,7 +237,7 @@ public class ThemeOverlayApplierTest extends SysuiTestCase { mock(FabricatedOverlay.class) }; mManager.applyCurrentUserOverlays(ALL_CATEGORIES_MAP, pendingCreation, - TEST_USER.getIdentifier(), TEST_USER_HANDLES); + TEST_USER.getIdentifier(), TEST_USER_HANDLES, mOnOverlaysApplied); for (FabricatedOverlay overlay : pendingCreation) { verify(mTransactionBuilder).registerFabricatedOverlay(eq(overlay)); @@ -247,7 +251,7 @@ public class ThemeOverlayApplierTest extends SysuiTestCase { categoryToPackage.remove(OVERLAY_CATEGORY_ICON_ANDROID); mManager.applyCurrentUserOverlays(categoryToPackage, null, TEST_USER.getIdentifier(), - TEST_USER_HANDLES); + TEST_USER_HANDLES, mOnOverlaysApplied); for (OverlayIdentifier overlayPackage : categoryToPackage.values()) { verify(mTransactionBuilder).setEnabled(eq(overlayPackage), eq(true), @@ -264,7 +268,7 @@ public class ThemeOverlayApplierTest extends SysuiTestCase { @Test public void zeroCategoriesSpecified_allDisabled() { mManager.applyCurrentUserOverlays(Maps.newArrayMap(), null, TEST_USER.getIdentifier(), - TEST_USER_HANDLES); + TEST_USER_HANDLES, mOnOverlaysApplied); for (String category : THEME_CATEGORIES) { verify(mTransactionBuilder).setEnabled( @@ -279,7 +283,7 @@ public class ThemeOverlayApplierTest extends SysuiTestCase { categoryToPackage.put("blah.category", new OverlayIdentifier("com.example.blah.category")); mManager.applyCurrentUserOverlays(categoryToPackage, null, TEST_USER.getIdentifier(), - TEST_USER_HANDLES); + TEST_USER_HANDLES, mOnOverlaysApplied); verify(mTransactionBuilder, never()).setEnabled( eq(new OverlayIdentifier("com.example.blah.category")), eq(false), diff --git a/packages/SystemUI/tests/src/com/android/systemui/theme/ThemeOverlayControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/theme/ThemeOverlayControllerTest.java index 07d3fc20983f4..d3820f06c645e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/theme/ThemeOverlayControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/theme/ThemeOverlayControllerTest.java @@ -161,7 +161,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { ArgumentCaptor.forClass(Map.class); verify(mThemeOverlayApplier) - .applyCurrentUserOverlays(themeOverlays.capture(), any(), anyInt(), any()); + .applyCurrentUserOverlays(themeOverlays.capture(), any(), anyInt(), any(), any()); // Assert that we received the colors that we were expecting assertThat(themeOverlays.getValue().get(OVERLAY_CATEGORY_SYSTEM_PALETTE)) @@ -183,7 +183,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { mBroadcastReceiver.getValue().onReceive(null, new Intent(Intent.ACTION_WALLPAPER_CHANGED)); mColorsListener.getValue().onColorsChanged(new WallpaperColors(Color.valueOf(Color.BLACK), null, null), WallpaperManager.FLAG_SYSTEM); - verify(mThemeOverlayApplier).applyCurrentUserOverlays(any(), any(), anyInt(), any()); + verify(mThemeOverlayApplier).applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); } @Test @@ -204,7 +204,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { ArgumentCaptor.forClass(Map.class); verify(mThemeOverlayApplier) - .applyCurrentUserOverlays(themeOverlays.capture(), any(), anyInt(), any()); + .applyCurrentUserOverlays(themeOverlays.capture(), any(), anyInt(), any(), any()); // Assert that we received the colors that we were expecting assertThat(themeOverlays.getValue().get(OVERLAY_CATEGORY_SYSTEM_PALETTE)) @@ -240,7 +240,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { .isFalse(); verify(mThemeOverlayApplier) - .applyCurrentUserOverlays(any(), any(), anyInt(), any()); + .applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); } @Test @@ -270,7 +270,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { "android.theme.customization.color_both\":\"0")).isTrue(); verify(mThemeOverlayApplier) - .applyCurrentUserOverlays(any(), any(), anyInt(), any()); + .applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); } @Test @@ -300,7 +300,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { "android.theme.customization.color_both\":\"1")).isTrue(); verify(mThemeOverlayApplier) - .applyCurrentUserOverlays(any(), any(), anyInt(), any()); + .applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); } @Test @@ -327,7 +327,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { assertThat(updatedSetting.getValue().contains("android.theme.customization.color_index")) .isFalse(); verify(mThemeOverlayApplier) - .applyCurrentUserOverlays(any(), any(), anyInt(), any()); + .applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); } @Test @@ -354,7 +354,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { assertThat(updatedSetting.getValue().contains("android.theme.customization.color_index")) .isFalse(); verify(mThemeOverlayApplier) - .applyCurrentUserOverlays(any(), any(), anyInt(), any()); + .applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); } @Test @@ -382,7 +382,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { eq(Settings.Secure.THEME_CUSTOMIZATION_OVERLAY_PACKAGES), updatedSetting.capture()); verify(mThemeOverlayApplier) - .applyCurrentUserOverlays(any(), any(), anyInt(), any()); + .applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); } @Test @@ -411,14 +411,14 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { verify(mThemeOverlayApplier, never()) - .applyCurrentUserOverlays(any(), any(), anyInt(), any()); + .applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); } @Test public void onProfileAdded_setsTheme() { mBroadcastReceiver.getValue().onReceive(null, new Intent(Intent.ACTION_MANAGED_PROFILE_ADDED)); - verify(mThemeOverlayApplier).applyCurrentUserOverlays(any(), any(), anyInt(), any()); + verify(mThemeOverlayApplier).applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); } @Test @@ -428,7 +428,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { mBroadcastReceiver.getValue().onReceive(null, new Intent(Intent.ACTION_MANAGED_PROFILE_ADDED)); verify(mThemeOverlayApplier) - .applyCurrentUserOverlays(any(), any(), anyInt(), any()); + .applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); } @Test @@ -438,7 +438,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { mBroadcastReceiver.getValue().onReceive(null, new Intent(Intent.ACTION_MANAGED_PROFILE_ADDED)); verify(mThemeOverlayApplier, never()) - .applyCurrentUserOverlays(any(), any(), anyInt(), any()); + .applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); } @Test @@ -450,7 +450,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { Color.valueOf(Color.BLUE), null); mColorsListener.getValue().onColorsChanged(mainColors, WallpaperManager.FLAG_SYSTEM); - verify(mThemeOverlayApplier).applyCurrentUserOverlays(any(), any(), anyInt(), any()); + verify(mThemeOverlayApplier).applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); // Regression test: null events should not reset the internal state and allow colors to be // applied again. @@ -458,11 +458,11 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { mBroadcastReceiver.getValue().onReceive(null, new Intent(Intent.ACTION_WALLPAPER_CHANGED)); mColorsListener.getValue().onColorsChanged(null, WallpaperManager.FLAG_SYSTEM); verify(mThemeOverlayApplier, never()).applyCurrentUserOverlays(any(), any(), anyInt(), - any()); + any(), any()); mColorsListener.getValue().onColorsChanged(new WallpaperColors(Color.valueOf(Color.GREEN), null, null), WallpaperManager.FLAG_SYSTEM); verify(mThemeOverlayApplier, never()).applyCurrentUserOverlays(any(), any(), anyInt(), - any()); + any(), any()); } @Test @@ -499,7 +499,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { verify(mDeviceProvisionedController).addCallback(mDeviceProvisionedListener.capture()); // Colors were applied during controller initialization. - verify(mThemeOverlayApplier).applyCurrentUserOverlays(any(), any(), anyInt(), any()); + verify(mThemeOverlayApplier).applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); clearInvocations(mThemeOverlayApplier); } @@ -533,7 +533,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { verify(mDeviceProvisionedController).addCallback(mDeviceProvisionedListener.capture()); // Colors were applied during controller initialization. - verify(mThemeOverlayApplier).applyCurrentUserOverlays(any(), any(), anyInt(), any()); + verify(mThemeOverlayApplier).applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); clearInvocations(mThemeOverlayApplier); WallpaperColors mainColors = new WallpaperColors(Color.valueOf(Color.RED), @@ -542,12 +542,12 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { // Defers event because we already have initial colors. verify(mThemeOverlayApplier, never()) - .applyCurrentUserOverlays(any(), any(), anyInt(), any()); + .applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); // Then event happens after setup phase is over. when(mDeviceProvisionedController.isCurrentUserSetup()).thenReturn(true); mDeviceProvisionedListener.getValue().onUserSetupChanged(); - verify(mThemeOverlayApplier).applyCurrentUserOverlays(any(), any(), anyInt(), any()); + verify(mThemeOverlayApplier).applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); } @Test @@ -568,11 +568,11 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { Color.valueOf(Color.RED), null); mColorsListener.getValue().onColorsChanged(mainColors, WallpaperManager.FLAG_SYSTEM); verify(mThemeOverlayApplier, never()) - .applyCurrentUserOverlays(any(), any(), anyInt(), any()); + .applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); mWakefulnessLifecycle.dispatchFinishedGoingToSleep(); verify(mThemeOverlayApplier, never()) - .applyCurrentUserOverlays(any(), any(), anyInt(), any()); + .applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); } @Test @@ -592,10 +592,10 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { Color.valueOf(Color.RED), null); mColorsListener.getValue().onColorsChanged(mainColors, WallpaperManager.FLAG_SYSTEM); verify(mThemeOverlayApplier, never()) - .applyCurrentUserOverlays(any(), any(), anyInt(), any()); + .applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); mWakefulnessLifecycleObserver.getValue().onFinishedGoingToSleep(); - verify(mThemeOverlayApplier).applyCurrentUserOverlays(any(), any(), anyInt(), any()); + verify(mThemeOverlayApplier).applyCurrentUserOverlays(any(), any(), anyInt(), any(), any()); } @Test @@ -614,7 +614,7 @@ public class ThemeOverlayControllerTest extends SysuiTestCase { ArgumentCaptor.forClass(Map.class); verify(mThemeOverlayApplier) - .applyCurrentUserOverlays(themeOverlays.capture(), any(), anyInt(), any()); + .applyCurrentUserOverlays(themeOverlays.capture(), any(), anyInt(), any(), any()); // Assert that we received the colors that we were expecting assertThat(themeOverlays.getValue().get(OVERLAY_CATEGORY_SYSTEM_PALETTE))