From 31c837747ff1f9529b6780fb6e5ef724963e9ff0 Mon Sep 17 00:00:00 2001 From: Suprabh Shukla Date: Mon, 27 Jun 2022 14:01:16 -0700 Subject: [PATCH 01/16] Stop crashing the system on hitting the alarm limit Exempting the system as a runtime restart is not clearly better than extreme memory and computation pressure that can result from the originating spam. Callers in the system should guard against any spammy requests that lead them to create a lot of alarms. Test: Builds, boots and existing tests should pass. atest CtsAlarmManagerTestCases:UidCapTests atest FrameworksMockingServicesTests:AlarmManagerServiceTest Bug: 234441463 Change-Id: Id5e94d44ac9ab24870a8213ec7583da0f592a5ff (cherry picked from commit 3b9f3f4a0f5a661be65e287996cae8a4481a1453) Merged-In: Id5e94d44ac9ab24870a8213ec7583da0f592a5ff (cherry picked from commit 2100d275644575214a8e74457bb973b44a3bacff) Merged-In: Id5e94d44ac9ab24870a8213ec7583da0f592a5ff --- .../java/com/android/server/alarm/AlarmManagerService.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java index 0de0a1cf9c8e9..d6b246a9e2e39 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java @@ -112,6 +112,7 @@ import android.text.TextUtils; import android.text.format.DateFormat; import android.util.ArrayMap; import android.util.ArraySet; +import android.util.EventLog; import android.util.IndentingPrintWriter; import android.util.Log; import android.util.LongArrayQueue; @@ -2299,7 +2300,11 @@ public class AlarmManagerService extends SystemService { + " reached for uid: " + UserHandle.formatUid(callingUid) + ", callingPackage: " + callingPackage; Slog.w(TAG, errorMsg); - throw new IllegalStateException(errorMsg); + if (callingUid != Process.SYSTEM_UID) { + throw new IllegalStateException(errorMsg); + } else { + EventLog.writeEvent(0x534e4554, "234441463", -1, errorMsg); + } } setImplLocked(type, triggerAtTime, triggerElapsed, windowLength, interval, operation, directReceiver, listenerTag, flags, workSource, alarmClock, callingUid, From cf5893107728c328716350d9edb9b0d587fc3a46 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Tue, 19 Jul 2022 18:38:51 +0000 Subject: [PATCH 02/16] Revert "Revert "Limit the number of concurrently snoozed notifications"" This reverts commit 705019b5fcea333b3041a0a56a0c4e61c41e1b76. Reason for revert: adding merged in this time Merged-In: I005b43979d1c708fd505c8b33ae0c8cb03ddbb35 Bug: 234441463 Change-Id: I5ecfdf77e56d9a50ca5cc8031ce83313b8d4ee5d (cherry picked from commit e2ceb6acfae4daa6fb095ca4031324f9f4d0fa96) Merged-In: I5ecfdf77e56d9a50ca5cc8031ce83313b8d4ee5d --- .../NotificationManagerService.java | 19 +++-- .../server/notification/SnoozeHelper.java | 11 +++ .../NotificationManagerServiceTest.java | 85 ++++++++++++++++--- .../server/notification/SnoozeHelperTest.java | 17 ++++ 4 files changed, 117 insertions(+), 15 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index d1e0b0474b61e..88f543260871a 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -7030,6 +7030,7 @@ public class NotificationManagerService extends SystemService { @GuardedBy("mNotificationLock") void snoozeLocked(NotificationRecord r) { + final List recordsToSnooze = new ArrayList<>(); if (r.getSbn().isGroup()) { final List groupNotifications = findCurrentAndSnoozedGroupNotificationsLocked( @@ -7038,8 +7039,8 @@ public class NotificationManagerService extends SystemService { if (r.getNotification().isGroupSummary()) { // snooze all children for (int i = 0; i < groupNotifications.size(); i++) { - if (mKey != groupNotifications.get(i).getKey()) { - snoozeNotificationLocked(groupNotifications.get(i)); + if (!mKey.equals(groupNotifications.get(i).getKey())) { + recordsToSnooze.add(groupNotifications.get(i)); } } } else { @@ -7049,8 +7050,8 @@ public class NotificationManagerService extends SystemService { if (groupNotifications.size() == 2) { // snooze summary and the one child for (int i = 0; i < groupNotifications.size(); i++) { - if (mKey != groupNotifications.get(i).getKey()) { - snoozeNotificationLocked(groupNotifications.get(i)); + if (!mKey.equals(groupNotifications.get(i).getKey())) { + recordsToSnooze.add(groupNotifications.get(i)); } } } @@ -7058,7 +7059,15 @@ public class NotificationManagerService extends SystemService { } } // snooze the notification - snoozeNotificationLocked(r); + recordsToSnooze.add(r); + + if (mSnoozeHelper.canSnooze(recordsToSnooze.size())) { + for (int i = 0; i < recordsToSnooze.size(); i++) { + snoozeNotificationLocked(recordsToSnooze.get(i)); + } + } else { + Log.w(TAG, "Cannot snooze " + r.getKey() + ": too many snoozed notifications"); + } } diff --git a/services/core/java/com/android/server/notification/SnoozeHelper.java b/services/core/java/com/android/server/notification/SnoozeHelper.java index 7f265df3f416a..15d7c1e7a2102 100644 --- a/services/core/java/com/android/server/notification/SnoozeHelper.java +++ b/services/core/java/com/android/server/notification/SnoozeHelper.java @@ -62,6 +62,8 @@ import java.util.Set; public class SnoozeHelper { public static final int XML_SNOOZED_NOTIFICATION_VERSION = 1; + static final int CONCURRENT_SNOOZE_LIMIT = 500; + protected static final String XML_TAG_NAME = "snoozed-notifications"; private static final String XML_SNOOZED_NOTIFICATION = "notification"; @@ -135,6 +137,15 @@ public class SnoozeHelper { } } + protected boolean canSnooze(int numberToSnooze) { + synchronized (mLock) { + if ((mPackages.size() + numberToSnooze) > CONCURRENT_SNOOZE_LIMIT) { + return false; + } + } + return true; + } + @NonNull protected Long getSnoozeTimeForUnpostedNotification(int userId, String pkg, String key) { Long time = null; diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index 22721a1bcc92a..b1b323b734bd2 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -3380,19 +3380,80 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { } @Test - public void testSnoozeRunnable_reSnoozeASingleSnoozedNotification() throws Exception { + public void testSnoozeRunnable_tooManySnoozed_singleNotification() { + final NotificationRecord notification = generateNotificationRecord( + mTestNotificationChannel, 1, null, true); + mService.addNotification(notification); + + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); + when(mSnoozeHelper.canSnooze(1)).thenReturn(false); + + NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = + mService.new SnoozeNotificationRunnable( + notification.getKey(), 100, null); + snoozeNotificationRunnable.run(); + + verify(mSnoozeHelper, never()).snooze(any(NotificationRecord.class), anyLong()); + assertThat(mService.getNotificationRecordCount()).isEqualTo(1); + } + + @Test + public void testSnoozeRunnable_tooManySnoozed_singleGroupChildNotification() { + final NotificationRecord notification = generateNotificationRecord( + mTestNotificationChannel, 1, "group", true); + final NotificationRecord notificationChild = generateNotificationRecord( + mTestNotificationChannel, 1, "group", false); + mService.addNotification(notification); + mService.addNotification(notificationChild); + + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); + when(mSnoozeHelper.canSnooze(2)).thenReturn(false); + + NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = + mService.new SnoozeNotificationRunnable( + notificationChild.getKey(), 100, null); + snoozeNotificationRunnable.run(); + + verify(mSnoozeHelper, never()).snooze(any(NotificationRecord.class), anyLong()); + assertThat(mService.getNotificationRecordCount()).isEqualTo(2); + } + + @Test + public void testSnoozeRunnable_tooManySnoozed_summaryNotification() { + final NotificationRecord notification = generateNotificationRecord( + mTestNotificationChannel, 1, "group", true); + final NotificationRecord notificationChild = generateNotificationRecord( + mTestNotificationChannel, 12, "group", false); + final NotificationRecord notificationChild2 = generateNotificationRecord( + mTestNotificationChannel, 13, "group", false); + mService.addNotification(notification); + mService.addNotification(notificationChild); + mService.addNotification(notificationChild2); + + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); + when(mSnoozeHelper.canSnooze(3)).thenReturn(false); + + NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = + mService.new SnoozeNotificationRunnable( + notification.getKey(), 100, null); + snoozeNotificationRunnable.run(); + + verify(mSnoozeHelper, never()).snooze(any(NotificationRecord.class), anyLong()); + assertThat(mService.getNotificationRecordCount()).isEqualTo(3); + } + + @Test + public void testSnoozeRunnable_reSnoozeASingleSnoozedNotification() { final NotificationRecord notification = generateNotificationRecord( mTestNotificationChannel, 1, null, true); mService.addNotification(notification); when(mSnoozeHelper.getNotification(any())).thenReturn(notification); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( notification.getKey(), 100, null); snoozeNotificationRunnable.run(); - NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable2 = - mService.new SnoozeNotificationRunnable( - notification.getKey(), 100, null); snoozeNotificationRunnable.run(); // snooze twice @@ -3400,19 +3461,17 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { } @Test - public void testSnoozeRunnable_reSnoozeASnoozedNotificationWithGroupKey() throws Exception { + public void testSnoozeRunnable_reSnoozeASnoozedNotificationWithGroupKey() { final NotificationRecord notification = generateNotificationRecord( mTestNotificationChannel, 1, "group", true); mService.addNotification(notification); when(mSnoozeHelper.getNotification(any())).thenReturn(notification); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( notification.getKey(), 100, null); snoozeNotificationRunnable.run(); - NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable2 = - mService.new SnoozeNotificationRunnable( - notification.getKey(), 100, null); snoozeNotificationRunnable.run(); // snooze twice @@ -3430,6 +3489,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { when(mSnoozeHelper.getNotification(any())).thenReturn(notification); when(mSnoozeHelper.getNotifications( anyString(), anyString(), anyInt())).thenReturn(new ArrayList<>()); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( @@ -3439,8 +3499,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { .thenReturn(new ArrayList<>(Arrays.asList(notification, notification2))); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable2 = mService.new SnoozeNotificationRunnable( - notification.getKey(), 100, null); - snoozeNotificationRunnable.run(); + notification2.getKey(), 100, null); + snoozeNotificationRunnable2.run(); // snooze twice verify(mSnoozeHelper, times(4)).snooze(any(NotificationRecord.class), anyLong()); @@ -3454,6 +3514,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mTestNotificationChannel, 2, "group", false); mService.addNotification(grouped); mService.addNotification(nonGrouped); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( @@ -3483,6 +3544,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mService.addNotification(parent); mService.addNotification(child); mService.addNotification(child2); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( @@ -3504,6 +3566,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mService.addNotification(parent); mService.addNotification(child); mService.addNotification(child2); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( @@ -3529,6 +3592,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mTestNotificationChannel, 2, "group", false); mService.addNotification(parent); mService.addNotification(child); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( @@ -3556,6 +3620,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { final NotificationRecord child = generateNotificationRecord( mTestNotificationChannel, 2, "group", false); mService.addNotification(child); + when(mSnoozeHelper.canSnooze(anyInt())).thenReturn(true); NotificationManagerService.SnoozeNotificationRunnable snoozeNotificationRunnable = mService.new SnoozeNotificationRunnable( diff --git a/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java index 2ae2ef7162a52..8bead57745487 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java @@ -15,6 +15,7 @@ */ package com.android.server.notification; +import static com.android.server.notification.SnoozeHelper.CONCURRENT_SNOOZE_LIMIT; import static com.android.server.notification.SnoozeHelper.EXTRA_KEY; import static junit.framework.Assert.assertEquals; @@ -280,6 +281,22 @@ public class SnoozeHelperTest extends UiServiceTestCase { UserHandle.USER_SYSTEM, r.getSbn().getPackageName(), r.getKey())); } + @Test + public void testSnoozeLimit() { + for (int i = 0; i < CONCURRENT_SNOOZE_LIMIT; i++ ) { + NotificationRecord r = getNotificationRecord("pkg", i, i+"", UserHandle.SYSTEM); + + assertTrue("cannot snooze record " + i, mSnoozeHelper.canSnooze(1)); + + if (i % 2 == 0) { + mSnoozeHelper.snooze(r, null); + } else { + mSnoozeHelper.snooze(r, 9000); + } + } + assertFalse(mSnoozeHelper.canSnooze(1)); + } + @Test public void testCancelByApp() throws Exception { NotificationRecord r = getNotificationRecord("pkg", 1, "one", UserHandle.SYSTEM); From 494599d3d7b251f80c71c4f6d73b84d64dc3fa1d Mon Sep 17 00:00:00 2001 From: Beth Thibodeau Date: Tue, 19 Jul 2022 21:29:29 -0500 Subject: [PATCH 03/16] Fix crash when loading album art by resource When the album art is loaded from the notification largeIcon it may be a resource rather than a bitmap, so it has to be loaded as a drawable. Bitmaps are still loaded using `WallpaperColors.fromBitmap` to avoid the extra steps needed by the `fromDrawable` method. Fixes: 239368697 Test: manual Test: atest MediaControlPanelTest Change-Id: I0ba9a4b0f4d308cc360a785b68cbabdcb2480690 (cherry picked from commit ef53f601f2b46dfaaef27d7fecb1712178e5c6ea) (cherry picked from commit d0c1f811af2b0d3c035644494676acebae1cff05) Merged-In: I0ba9a4b0f4d308cc360a785b68cbabdcb2480690 --- .../systemui/media/MediaControlPanel.java | 17 +++++++++++++++-- .../systemui/media/MediaControlPanelTest.kt | 13 +++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java index 33453a4c31d64..159784575b69a 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java +++ b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java @@ -631,9 +631,22 @@ public class MediaControlPanel { Drawable artwork; boolean isArtworkBound; Icon artworkIcon = data.getArtwork(); + WallpaperColors wallpaperColors = null; if (artworkIcon != null) { - WallpaperColors wallpaperColors = WallpaperColors - .fromBitmap(artworkIcon.getBitmap()); + if (artworkIcon.getType() == Icon.TYPE_BITMAP + || artworkIcon.getType() == Icon.TYPE_ADAPTIVE_BITMAP) { + // Avoids extra processing if this is already a valid bitmap + wallpaperColors = WallpaperColors + .fromBitmap(artworkIcon.getBitmap()); + } else { + Drawable artworkDrawable = artworkIcon.loadDrawable(mContext); + if (artworkDrawable != null) { + wallpaperColors = WallpaperColors + .fromDrawable(artworkIcon.loadDrawable(mContext)); + } + } + } + if (wallpaperColors != null) { mutableColorScheme = new ColorScheme(wallpaperColors, true, Style.CONTENT); artwork = getScaledBackground(artworkIcon, width, height); isArtworkBound = true; diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt index 241ed2443e6c0..6f4579bb14b5e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt @@ -546,6 +546,19 @@ public class MediaControlPanelTest : SysuiTestCase() { verify(albumView).setLayerType(View.LAYER_TYPE_HARDWARE, null) } + @Test + fun bindAlbumView_artUsesResource() { + val albumArt = Icon.createWithResource(context, R.drawable.ic_android) + val state = mediaData.copy(artwork = albumArt) + + player.attachPlayer(viewHolder) + player.bindPlayer(state, PACKAGE) + bgExecutor.runAllReady() + mainExecutor.runAllReady() + + verify(albumView).setImageDrawable(any(Drawable::class.java)) + } + @Test fun bindAlbumView_setAfterExecutors() { val bmp = Bitmap.createBitmap(10, 10, Bitmap.Config.ARGB_8888) From 95e7a3ee33d3665ec46570dd0b1a6db614384570 Mon Sep 17 00:00:00 2001 From: Jing Ji Date: Wed, 13 Jul 2022 18:16:52 -0700 Subject: [PATCH 04/16] Perform the user-allowlist check after all other fixed allowlist check ...In the app restriction controller. Bug: 238377411 Test: see b/238377411#comment8 Change-Id: I5beb161f46172d518f54f8e99f682116083a6ca4 Merged-In: I5beb161f46172d518f54f8e99f682116083a6ca4 (cherry picked from commit 89d9bf4aa23943c18856aecad832cf00bad464a5) (cherry picked from commit 26945bf8608a8d5e38bdda4a68e3d444eed03de0) Merged-In: I5beb161f46172d518f54f8e99f682116083a6ca4 --- .../server/am/AppRestrictionController.java | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/services/core/java/com/android/server/am/AppRestrictionController.java b/services/core/java/com/android/server/am/AppRestrictionController.java index 6076eb1f69eed..7a09ce722f942 100644 --- a/services/core/java/com/android/server/am/AppRestrictionController.java +++ b/services/core/java/com/android/server/am/AppRestrictionController.java @@ -2790,13 +2790,6 @@ public final class AppRestrictionController { if (isOnSystemDeviceIdleAllowlist(uid)) { return REASON_SYSTEM_ALLOW_LISTED; } - if (isOnDeviceIdleAllowlist(uid)) { - return REASON_ALLOWLISTED_PACKAGE; - } - final ActivityManagerInternal am = mInjector.getActivityManagerInternal(); - if (am.isAssociatedCompanionApp(UserHandle.getUserId(uid), uid)) { - return REASON_COMPANION_DEVICE_MANAGER; - } if (UserManager.isDeviceInDemoMode(mContext)) { return REASON_DEVICE_DEMO_MODE; } @@ -2805,6 +2798,7 @@ public final class AppRestrictionController { .hasUserRestriction(UserManager.DISALLOW_APPS_CONTROL, userId)) { return REASON_DISALLOW_APPS_CONTROL; } + final ActivityManagerInternal am = mInjector.getActivityManagerInternal(); if (am.isDeviceOwner(uid)) { return REASON_DEVICE_OWNER; } @@ -2822,14 +2816,9 @@ public final class AppRestrictionController { final AppOpsManager appOpsManager = mInjector.getAppOpsManager(); final PackageManagerInternal pm = mInjector.getPackageManagerInternal(); final AppStandbyInternal appStandbyInternal = mInjector.getAppStandbyInternal(); + // Check each packages to see if any of them is in the "fixed" exemption cases. for (String pkg : packages) { - if (appOpsManager.checkOpNoThrow(AppOpsManager.OP_ACTIVATE_VPN, - uid, pkg) == AppOpsManager.MODE_ALLOWED) { - return REASON_OP_ACTIVATE_VPN; - } else if (appOpsManager.checkOpNoThrow(AppOpsManager.OP_ACTIVATE_PLATFORM_VPN, - uid, pkg) == AppOpsManager.MODE_ALLOWED) { - return REASON_OP_ACTIVATE_PLATFORM_VPN; - } else if (isSystemModule(pkg)) { + if (isSystemModule(pkg)) { return REASON_SYSTEM_MODULE; } else if (isCarrierApp(pkg)) { return REASON_CARRIER_PRIVILEGED_APP; @@ -2843,6 +2832,16 @@ public final class AppRestrictionController { return REASON_ACTIVE_DEVICE_ADMIN; } } + // Loop the packages again, and check the user-configurable exemptions. + for (String pkg : packages) { + if (appOpsManager.checkOpNoThrow(AppOpsManager.OP_ACTIVATE_VPN, + uid, pkg) == AppOpsManager.MODE_ALLOWED) { + return REASON_OP_ACTIVATE_VPN; + } else if (appOpsManager.checkOpNoThrow(AppOpsManager.OP_ACTIVATE_PLATFORM_VPN, + uid, pkg) == AppOpsManager.MODE_ALLOWED) { + return REASON_OP_ACTIVATE_PLATFORM_VPN; + } + } } if (isRoleHeldByUid(RoleManager.ROLE_DIALER, uid)) { return REASON_ROLE_DIALER; @@ -2850,6 +2849,12 @@ public final class AppRestrictionController { if (isRoleHeldByUid(RoleManager.ROLE_EMERGENCY, uid)) { return REASON_ROLE_EMERGENCY; } + if (isOnDeviceIdleAllowlist(uid)) { + return REASON_ALLOWLISTED_PACKAGE; + } + if (am.isAssociatedCompanionApp(UserHandle.getUserId(uid), uid)) { + return REASON_COMPANION_DEVICE_MANAGER; + } return REASON_DENIED; } From aefdf086664451cee3fd003ae2b9149546027f3c Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Thu, 21 Jul 2022 16:15:11 +0000 Subject: [PATCH 05/16] Fix Notification redaction when power cycling a non-dozing device while occluded. This issue was originally raised in the S timeline, but had already been fixed by the refactor to use UnlockedScreenOffAnimationController, which called updateIsKeyguard(/*force*/ true) from onFinishedWakingUp(). This solved the problem of re-triggering the redaction, but it also intriduced a new bug where the keyguard could end up briefly showing on top of the occluding activity when AOD was supported but off. As a result, they limited the call to when the AOD was on (and animations were controlling, etc). This CL uses the opposite check to make sure we recalcualte redaction (and only redaction, not the whole keyguard) when waking up while occluded. We also needed to make sure that we rerun the notification pipeline when updating public information so that any necessary public views are sure to inflate. That rerun has been limited in scope to conditions where the public mode information has detectably changed. Bug: 189575031 Bug: 237349699 Bug: 238990302 Bug: 239828798 Test: CTS Verifier NotificationPrivacyTest on emulator, AOD off, AOD on Merged-In: I95443ee6b77377aceb54b983d34131628027da9b Change-Id: I95443ee6b77377aceb54b983d34131628027da9b (cherry picked from commit 09d333e09cf0ef418c0bd32581d94aa01ade4d9b) Merged-In: I95443ee6b77377aceb54b983d34131628027da9b --- ...NotificationLockscreenUserManagerImpl.java | 7 +++- ...tificationStackScrollLayoutController.java | 7 ++++ .../statusbar/phone/CentralSurfacesImpl.java | 12 +++++++ ...NotificationLockscreenUserManagerTest.java | 35 +++++++++++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java index 38a208b72edca..f4ca7edb146d3 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java @@ -642,6 +642,8 @@ public class NotificationLockscreenUserManagerImpl implements // - device keyguard is shown in secure mode; // - profile is locked with a work challenge. SparseArray currentProfiles = getCurrentProfiles(); + SparseBooleanArray oldPublicModes = mLockscreenPublicMode.clone(); + SparseBooleanArray oldWorkChallenges = mUsersWithSeparateWorkChallenge.clone(); mUsersWithSeparateWorkChallenge.clear(); for (int i = currentProfiles.size() - 1; i >= 0; i--) { final int userId = currentProfiles.valueAt(i).id; @@ -660,7 +662,10 @@ public class NotificationLockscreenUserManagerImpl implements } getEntryManager().updateNotifications("NotificationLockscreenUserManager.updatePublicMode"); // TODO(b/234738798): Migrate KeyguardNotificationVisibilityProvider to use this listener - // notifyNotificationStateChanged(); + if (!mLockscreenPublicMode.equals(oldPublicModes) + || !mUsersWithSeparateWorkChallenge.equals(oldWorkChallenges)) { + notifyNotificationStateChanged(); + } } @Override diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutController.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutController.java index 010e6cf908174..e5f8424a2a24c 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutController.java @@ -338,6 +338,13 @@ public class NotificationStackScrollLayoutController { } }; + /** + * Recalculate sensitiveness without animation; called when waking up while keyguard occluded. + */ + public void updateSensitivenessForOccludedWakeup() { + mView.updateSensitiveness(false, mLockscreenUserManager.isAnyProfilePublicMode()); + } + /** * Set the overexpansion of the panel to be applied to the view. */ diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java index 7e57dd452cb8c..0e35cbc1aaf55 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java @@ -3687,6 +3687,18 @@ public class CentralSurfacesImpl extends CoreStartable implements public void onFinishedWakingUp() { mWakeUpCoordinator.setFullyAwake(true); mWakeUpCoordinator.setWakingUp(false); + if (mKeyguardStateController.isOccluded() + && !mDozeParameters.canControlUnlockedScreenOff()) { + // When the keyguard is occluded we don't use the KEYGUARD state which would + // normally cause these redaction updates. If AOD is on, the KEYGUARD state is used + // to show the doze, AND UnlockedScreenOffAnimationController.onFinishedWakingUp() + // would force a KEYGUARD state that would take care of recalculating redaction. + // So if AOD is off or unsupported we need to trigger these updates at screen on + // when the keyguard is occluded. + mLockscreenUserManager.updatePublicMode(); + mNotificationPanelViewController.getNotificationStackScrollLayoutController() + .updateSensitivenessForOccludedWakeup(); + } if (mLaunchCameraWhenFinishedWaking) { mNotificationPanelViewController.launchCamera( false /* animate */, mLastCameraLaunchSource); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java index 7687d12045414..dd2b66765fae6 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java @@ -29,7 +29,9 @@ import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertTrue; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.clearInvocations; 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.when; @@ -59,6 +61,7 @@ import com.android.systemui.broadcast.BroadcastDispatcher; import com.android.systemui.dump.DumpManager; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.NotificationLockscreenUserManager.KeyguardNotificationSuppressor; +import com.android.systemui.statusbar.NotificationLockscreenUserManager.NotificationStateChangedListener; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder; @@ -324,6 +327,38 @@ public class NotificationLockscreenUserManagerTest extends SysuiTestCase { assertTrue(mLockscreenUserManager.isLockscreenPublicMode(mCurrentUser.id)); } + @Test + public void testUpdateIsPublicMode() { + when(mKeyguardStateController.isMethodSecure()).thenReturn(true); + + NotificationStateChangedListener listener = mock(NotificationStateChangedListener.class); + mLockscreenUserManager.addNotificationStateChangedListener(listener); + mLockscreenUserManager.mCurrentProfiles.append(0, mock(UserInfo.class)); + + // first call explicitly sets user 0 to not public; notifies + mLockscreenUserManager.updatePublicMode(); + assertFalse(mLockscreenUserManager.isLockscreenPublicMode(0)); + verify(listener).onNotificationStateChanged(); + clearInvocations(listener); + + // calling again has no changes; does not notify + mLockscreenUserManager.updatePublicMode(); + assertFalse(mLockscreenUserManager.isLockscreenPublicMode(0)); + verify(listener, never()).onNotificationStateChanged(); + + // Calling again with keyguard now showing makes user 0 public; notifies + when(mKeyguardStateController.isShowing()).thenReturn(true); + mLockscreenUserManager.updatePublicMode(); + assertTrue(mLockscreenUserManager.isLockscreenPublicMode(0)); + verify(listener).onNotificationStateChanged(); + clearInvocations(listener); + + // calling again has no changes; does not notify + mLockscreenUserManager.updatePublicMode(); + assertTrue(mLockscreenUserManager.isLockscreenPublicMode(0)); + verify(listener, never()).onNotificationStateChanged(); + } + @Test public void testShowSilentNotifications_settingSaysShow() { mSettings.putInt(Settings.Secure.LOCK_SCREEN_SHOW_NOTIFICATIONS, 1); From 9194bce0d58093bbb0aa77c6050bb9a15d9dbc2a Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Mon, 18 Jul 2022 13:28:21 +0000 Subject: [PATCH 06/16] Block FullScreenIntent while device is in use if notification has a silencing GroupAlertBehavior. Bug: 231322873 Test: atest NotificationInterruptStateProviderImplTest Merged-In: Id82d20c9f1f2001400871b5381f52b40fbdf81c5 Change-Id: Id82d20c9f1f2001400871b5381f52b40fbdf81c5 (cherry picked from commit d107a27fe5c45065547a0257d3f995e23ba01e32) Merged-In: Id82d20c9f1f2001400871b5381f52b40fbdf81c5 --- .../NotificationInterruptLogger.kt | 29 ++++ ...otificationInterruptStateProviderImpl.java | 74 ++++++++-- ...icationInterruptStateProviderImplTest.java | 138 ++++++++++++++++++ 3 files changed, 231 insertions(+), 10 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptLogger.kt index 6c99e3adb73ec..bef3d5095cb7a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptLogger.kt @@ -20,8 +20,10 @@ import android.service.notification.StatusBarNotification import com.android.systemui.log.LogBuffer import com.android.systemui.log.LogLevel.DEBUG import com.android.systemui.log.LogLevel.INFO +import com.android.systemui.log.LogLevel.WARNING import com.android.systemui.log.dagger.NotificationHeadsUpLog import com.android.systemui.log.dagger.NotificationLog +import com.android.systemui.statusbar.notification.collection.NotificationEntry import javax.inject.Inject class NotificationInterruptLogger @Inject constructor( @@ -212,6 +214,33 @@ class NotificationInterruptLogger @Inject constructor( }) } + fun logNoFullscreen(entry: NotificationEntry, reason: String) { + hunBuffer.log(TAG, DEBUG, { + str1 = entry.key + str2 = reason + }, { + "No FullScreenIntent: $str2: $str1" + }) + } + + fun logNoFullscreenWarning(entry: NotificationEntry, reason: String) { + hunBuffer.log(TAG, WARNING, { + str1 = entry.key + str2 = reason + }, { + "No FullScreenIntent: WARNING: $str2: $str1" + }) + } + + fun logFullscreen(entry: NotificationEntry, reason: String) { + hunBuffer.log(TAG, DEBUG, { + str1 = entry.key + str2 = reason + }, { + "FullScreenIntent: $str2: $str1" + }) + } + fun keyguardHideNotification(key: String) { hunBuffer.log(TAG, DEBUG, { str1 = key diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImpl.java index e210f193b0a1d..e1ddbd23e51af 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImpl.java @@ -177,9 +177,69 @@ public class NotificationInterruptStateProviderImpl implements NotificationInter */ @Override public boolean shouldLaunchFullScreenIntentWhenAdded(NotificationEntry entry) { - return entry.getSbn().getNotification().fullScreenIntent != null - && (!shouldHeadsUp(entry) - || mStatusBarStateController.getState() == StatusBarState.KEYGUARD); + if (entry.getSbn().getNotification().fullScreenIntent == null) { + return false; + } + + // Never show FSI when suppressed by DND + if (entry.shouldSuppressFullScreenIntent()) { + mLogger.logNoFullscreen(entry, "Suppressed by DND"); + return false; + } + + // Never show FSI if importance is not HIGH + if (entry.getImportance() < NotificationManager.IMPORTANCE_HIGH) { + mLogger.logNoFullscreen(entry, "Not important enough"); + return false; + } + + // If the notification has suppressive GroupAlertBehavior, block FSI and warn. + StatusBarNotification sbn = entry.getSbn(); + if (sbn.isGroup() && sbn.getNotification().suppressAlertingDueToGrouping()) { + // b/231322873: Detect and report an event when a notification has both an FSI and a + // suppressive groupAlertBehavior, and now correctly block the FSI from firing. + final int uid = entry.getSbn().getUid(); + android.util.EventLog.writeEvent(0x534e4554, "231322873", uid, "groupAlertBehavior"); + mLogger.logNoFullscreenWarning(entry, "GroupAlertBehavior will prevent HUN"); + return false; + } + + // If the screen is off, then launch the FullScreenIntent + if (!mPowerManager.isInteractive()) { + mLogger.logFullscreen(entry, "Device is not interactive"); + return true; + } + + // If the device is currently dreaming, then launch the FullScreenIntent + if (isDreaming()) { + mLogger.logFullscreen(entry, "Device is dreaming"); + return true; + } + + // If the keyguard is showing, then launch the FullScreenIntent + if (mStatusBarStateController.getState() == StatusBarState.KEYGUARD) { + mLogger.logFullscreen(entry, "Keyguard is showing"); + return true; + } + + // If the notification should HUN, then we don't need FSI + if (shouldHeadsUp(entry)) { + mLogger.logNoFullscreen(entry, "Expected to HUN"); + return false; + } + + // If the notification won't HUN for some other reason (DND/snooze/etc), launch FSI. + mLogger.logFullscreen(entry, "Expected not to HUN"); + return true; + } + + private boolean isDreaming() { + try { + return mDreamManager.isDreaming(); + } catch (RemoteException e) { + Log.e(TAG, "Failed to query dream manager.", e); + return false; + } } private boolean shouldHeadsUpWhenAwake(NotificationEntry entry) { @@ -219,13 +279,7 @@ public class NotificationInterruptStateProviderImpl implements NotificationInter return false; } - boolean isDreaming = false; - try { - isDreaming = mDreamManager.isDreaming(); - } catch (RemoteException e) { - Log.e(TAG, "Failed to query dream manager.", e); - } - boolean inUse = mPowerManager.isScreenOn() && !isDreaming; + boolean inUse = mPowerManager.isScreenOn() && !isDreaming(); if (!inUse) { mLogger.logNoHeadsUpNotInUse(sbn); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImplTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImplTest.java index 90627cb0fa78f..25add0da4e71b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImplTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImplTest.java @@ -24,6 +24,7 @@ import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_AMBIENT; import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_PEEK; import static com.android.systemui.statusbar.NotificationEntryHelper.modifyRanking; +import static com.android.systemui.statusbar.StatusBarState.KEYGUARD; import static com.android.systemui.statusbar.StatusBarState.SHADE; import static com.google.common.truth.Truth.assertThat; @@ -31,6 +32,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -95,6 +97,8 @@ public class NotificationInterruptStateProviderImplTest extends SysuiTestCase { NotifPipelineFlags mFlags; @Mock KeyguardNotificationVisibilityProvider mKeyguardNotificationVisibilityProvider; + @Mock + PendingIntent mPendingIntent; private NotificationInterruptStateProviderImpl mNotifInterruptionStateProvider; @@ -422,6 +426,122 @@ public class NotificationInterruptStateProviderImplTest extends SysuiTestCase { assertThat(mNotifInterruptionStateProvider.shouldHeadsUp(entry)).isFalse(); } + @Test + public void testShouldNotFullScreen_notPendingIntent() throws RemoteException { + NotificationEntry entry = createNotification(IMPORTANCE_HIGH); + when(mPowerManager.isInteractive()).thenReturn(true); + when(mDreamManager.isDreaming()).thenReturn(false); + when(mStatusBarStateController.getState()).thenReturn(SHADE); + + assertThat(mNotifInterruptionStateProvider.shouldLaunchFullScreenIntentWhenAdded(entry)) + .isFalse(); + verify(mLogger, never()).logNoFullscreen(any(), any()); + verify(mLogger, never()).logNoFullscreenWarning(any(), any()); + verify(mLogger, never()).logFullscreen(any(), any()); + } + + @Test + public void testShouldNotFullScreen_notHighImportance() throws RemoteException { + NotificationEntry entry = createFsiNotification(IMPORTANCE_DEFAULT, /* silenced */ false); + when(mPowerManager.isInteractive()).thenReturn(true); + when(mDreamManager.isDreaming()).thenReturn(false); + when(mStatusBarStateController.getState()).thenReturn(SHADE); + + assertThat(mNotifInterruptionStateProvider.shouldLaunchFullScreenIntentWhenAdded(entry)) + .isFalse(); + verify(mLogger).logNoFullscreen(entry, "Not important enough"); + verify(mLogger, never()).logNoFullscreenWarning(any(), any()); + verify(mLogger, never()).logFullscreen(any(), any()); + } + + @Test + public void testShouldNotFullScreen_isGroupAlertSilenced() throws RemoteException { + NotificationEntry entry = createFsiNotification(IMPORTANCE_HIGH, /* silenced */ true); + when(mPowerManager.isInteractive()).thenReturn(false); + when(mDreamManager.isDreaming()).thenReturn(true); + when(mStatusBarStateController.getState()).thenReturn(KEYGUARD); + + assertThat(mNotifInterruptionStateProvider.shouldLaunchFullScreenIntentWhenAdded(entry)) + .isFalse(); + verify(mLogger, never()).logNoFullscreen(any(), any()); + verify(mLogger).logNoFullscreenWarning(entry, "GroupAlertBehavior will prevent HUN"); + verify(mLogger, never()).logFullscreen(any(), any()); + } + + @Test + public void testShouldFullScreen_notInteractive() throws RemoteException { + NotificationEntry entry = createFsiNotification(IMPORTANCE_HIGH, /* silenced */ false); + when(mPowerManager.isInteractive()).thenReturn(false); + when(mDreamManager.isDreaming()).thenReturn(false); + when(mStatusBarStateController.getState()).thenReturn(SHADE); + + assertThat(mNotifInterruptionStateProvider.shouldLaunchFullScreenIntentWhenAdded(entry)) + .isTrue(); + verify(mLogger, never()).logNoFullscreen(any(), any()); + verify(mLogger, never()).logNoFullscreenWarning(any(), any()); + verify(mLogger).logFullscreen(entry, "Device is not interactive"); + } + + @Test + public void testShouldFullScreen_isDreaming() throws RemoteException { + NotificationEntry entry = createFsiNotification(IMPORTANCE_HIGH, /* silenced */ false); + when(mPowerManager.isInteractive()).thenReturn(true); + when(mDreamManager.isDreaming()).thenReturn(true); + when(mStatusBarStateController.getState()).thenReturn(SHADE); + + assertThat(mNotifInterruptionStateProvider.shouldLaunchFullScreenIntentWhenAdded(entry)) + .isTrue(); + verify(mLogger, never()).logNoFullscreen(any(), any()); + verify(mLogger, never()).logNoFullscreenWarning(any(), any()); + verify(mLogger).logFullscreen(entry, "Device is dreaming"); + } + + @Test + public void testShouldFullScreen_onKeyguard() throws RemoteException { + NotificationEntry entry = createFsiNotification(IMPORTANCE_HIGH, /* silenced */ false); + when(mPowerManager.isInteractive()).thenReturn(true); + when(mDreamManager.isDreaming()).thenReturn(false); + when(mStatusBarStateController.getState()).thenReturn(KEYGUARD); + + assertThat(mNotifInterruptionStateProvider.shouldLaunchFullScreenIntentWhenAdded(entry)) + .isTrue(); + verify(mLogger, never()).logNoFullscreen(any(), any()); + verify(mLogger, never()).logNoFullscreenWarning(any(), any()); + verify(mLogger).logFullscreen(entry, "Keyguard is showing"); + } + + @Test + public void testShouldNotFullScreen_willHun() throws RemoteException { + NotificationEntry entry = createFsiNotification(IMPORTANCE_HIGH, /* silenced */ false); + when(mPowerManager.isInteractive()).thenReturn(true); + when(mPowerManager.isScreenOn()).thenReturn(true); + when(mDreamManager.isDreaming()).thenReturn(false); + when(mStatusBarStateController.getState()).thenReturn(SHADE); + + assertThat(mNotifInterruptionStateProvider.shouldLaunchFullScreenIntentWhenAdded(entry)) + .isFalse(); + verify(mLogger).logNoFullscreen(entry, "Expected to HUN"); + verify(mLogger, never()).logNoFullscreenWarning(any(), any()); + verify(mLogger, never()).logFullscreen(any(), any()); + } + + @Test + public void testShouldFullScreen_packageSnoozed() throws RemoteException { + NotificationEntry entry = createFsiNotification(IMPORTANCE_HIGH, /* silenced */ false); + when(mPowerManager.isInteractive()).thenReturn(true); + when(mPowerManager.isScreenOn()).thenReturn(true); + when(mDreamManager.isDreaming()).thenReturn(false); + when(mStatusBarStateController.getState()).thenReturn(SHADE); + when(mHeadsUpManager.isSnoozed("a")).thenReturn(true); + + assertThat(mNotifInterruptionStateProvider.shouldLaunchFullScreenIntentWhenAdded(entry)) + .isTrue(); + verify(mLogger).logNoHeadsUpPackageSnoozed(entry.getSbn()); + verify(mLogger, never()).logNoFullscreen(any(), any()); + verify(mLogger, never()).logNoFullscreenWarning(any(), any()); + verify(mLogger).logFullscreen(entry, "Expected not to HUN"); + } + /** * Bubbles can happen. */ @@ -526,6 +646,10 @@ public class NotificationInterruptStateProviderImplTest extends SysuiTestCase { .setContentText("content text") .build(); + return createNotification(importance, n); + } + + private NotificationEntry createNotification(int importance, Notification n) { return new NotificationEntryBuilder() .setPkg("a") .setOpPkg("a") @@ -536,6 +660,20 @@ public class NotificationInterruptStateProviderImplTest extends SysuiTestCase { .build(); } + private NotificationEntry createFsiNotification(int importance, boolean silent) { + Notification n = new Notification.Builder(getContext(), "a") + .setContentTitle("title") + .setContentText("content text") + .setFullScreenIntent(mPendingIntent, true) + .setGroup("fsi") + .setGroupAlertBehavior(silent + ? Notification.GROUP_ALERT_SUMMARY + : Notification.GROUP_ALERT_ALL) + .build(); + + return createNotification(importance, n); + } + private final NotificationInterruptSuppressor mSuppressAwakeHeadsUp = new NotificationInterruptSuppressor() { From e5dd8be748c76c11615050c610dfc1fae73ad4a4 Mon Sep 17 00:00:00 2001 From: Evan Rosky Date: Wed, 3 Aug 2022 11:48:33 -0700 Subject: [PATCH 07/16] Strip transition information from activityoptions when sent to app The implementation of shared-element transitions takes the ActivityOptions from the calling activity and sends them to another activity. This means that any sensitive information passed into ActivityManager via ActivityOptions can make its way to an unrelated app. Recently a RemoteTransition object was added which includes some sensitive information. This CL strips the sensitive information from the activity options before sending it to anonther app. Bug: 237290578 Test: atest ActivityManagerTest#testActivityManager_stripTransitionFromActivityOptions Change-Id: Ifa08fc195698f02bf70ca386178c67f6ba4a14ea (cherry picked from commit 0d03e6f1fc66fefb5409ac93ff49fa922f81664c) Merged-In: Ifa08fc195698f02bf70ca386178c67f6ba4a14ea --- core/java/android/app/ActivityOptions.java | 5 +++++ services/core/java/com/android/server/wm/ActivityRecord.java | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/core/java/android/app/ActivityOptions.java b/core/java/android/app/ActivityOptions.java index 2eebc01ccc04c..0ba1614ab6c5d 100644 --- a/core/java/android/app/ActivityOptions.java +++ b/core/java/android/app/ActivityOptions.java @@ -1441,6 +1441,11 @@ public class ActivityOptions extends ComponentOptions { return mRemoteTransition; } + /** @hide */ + public void setRemoteTransition(@Nullable RemoteTransition remoteTransition) { + mRemoteTransition = remoteTransition; + } + /** @hide */ public static ActivityOptions fromBundle(Bundle bOptions) { return bOptions != null ? new ActivityOptions(bOptions) : null; diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index ab936a6954d66..83687e9ebccdc 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -4884,8 +4884,12 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A ActivityOptions takeOptions() { if (DEBUG_TRANSITION) Slog.i(TAG, "Taking options for " + this + " callers=" + Debug.getCallers(6)); + if (mPendingOptions == null) return null; final ActivityOptions opts = mPendingOptions; mPendingOptions = null; + // Strip sensitive information from options before sending it to app. + opts.setRemoteTransition(null); + opts.setRemoteAnimationAdapter(null); return opts; } From 9a186a4de35312ee77319f1246b893e4f6fb1daf Mon Sep 17 00:00:00 2001 From: Aseem Kumar Date: Mon, 21 Mar 2022 20:35:20 -0700 Subject: [PATCH 08/16] Move accountname and typeName length check from Account.java to AccountManagerService. Bug: 169762606 Test: atest AccountManagerServiceTest Change-Id: I80fabf3a64c55837db98ff316e7e5420129c001b (cherry picked from commit 0adcadb0b28310bac568def4da2cbaf16843bcea) (cherry picked from commit cf62c760d4c002f562ddd5f372abe5bccda8a6ad) Merged-In: I80fabf3a64c55837db98ff316e7e5420129c001b --- core/java/android/accounts/Account.java | 7 ------- .../accounts/AccountManagerService.java | 12 +++++++++++ .../accounts/AccountManagerServiceTest.java | 21 +++++++++++++++++++ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/core/java/android/accounts/Account.java b/core/java/android/accounts/Account.java index e6cdcc0ee7420..0d6a07938e959 100644 --- a/core/java/android/accounts/Account.java +++ b/core/java/android/accounts/Account.java @@ -31,7 +31,6 @@ import android.util.Log; import com.android.internal.annotations.GuardedBy; -import java.util.Objects; import java.util.Set; /** @@ -87,12 +86,6 @@ public class Account implements Parcelable { if (TextUtils.isEmpty(type)) { throw new IllegalArgumentException("the type must not be empty: " + type); } - if (name.length() > 200) { - throw new IllegalArgumentException("account name is longer than 200 characters"); - } - if (type.length() > 200) { - throw new IllegalArgumentException("account type is longer than 200 characters"); - } this.name = name; this.type = type; this.accessId = accessId; diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java index b059cc7e2aa2d..2c465f44aa99e 100644 --- a/services/core/java/com/android/server/accounts/AccountManagerService.java +++ b/services/core/java/com/android/server/accounts/AccountManagerService.java @@ -1820,6 +1820,14 @@ public class AccountManagerService if (account == null) { return false; } + if (account.name != null && account.name.length() > 200) { + Log.w(TAG, "Account cannot be added - Name longer than 200 chars"); + return false; + } + if (account.type != null && account.type.length() > 200) { + Log.w(TAG, "Account cannot be added - Name longer than 200 chars"); + return false; + } if (!isLocalUnlockedUser(accounts.userId)) { Log.w(TAG, "Account " + account.toSafeString() + " cannot be added - user " + accounts.userId + " is locked. callingUid=" + callingUid); @@ -2065,6 +2073,10 @@ public class AccountManagerService + ", pid " + Binder.getCallingPid()); } if (accountToRename == null) throw new IllegalArgumentException("account is null"); + if (newName != null && newName.length() > 200) { + Log.e(TAG, "renameAccount failed - account name longer than 200"); + throw new IllegalArgumentException("account name longer than 200"); + } int userId = UserHandle.getCallingUserId(); if (!isAccountManagedByCaller(accountToRename.type, callingUid, userId)) { String msg = String.format( diff --git a/services/tests/servicestests/src/com/android/server/accounts/AccountManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/accounts/AccountManagerServiceTest.java index d5c5745d6680f..30ec1632a6220 100644 --- a/services/tests/servicestests/src/com/android/server/accounts/AccountManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/accounts/AccountManagerServiceTest.java @@ -39,6 +39,7 @@ import android.accounts.IAccountManagerResponse; import android.app.AppOpsManager; import android.app.PropertyInvalidatedCache; import android.app.INotificationManager; +import android.app.PropertyInvalidatedCache; import android.app.admin.DevicePolicyManager; import android.app.admin.DevicePolicyManagerInternal; import android.content.BroadcastReceiver; @@ -252,6 +253,26 @@ public class AccountManagerServiceTest extends AndroidTestCase { assertEquals(a31, accounts[1]); } + @SmallTest + public void testCheckAddAccountLongName() throws Exception { + unlockSystemUser(); + String longString = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaa"; + Account a11 = new Account(longString, AccountManagerServiceTestFixtures.ACCOUNT_TYPE_1); + + mAms.addAccountExplicitly( + a11, /* password= */ "p11", /* extras= */ null, /* callerPackage= */ null); + + String[] list = new String[]{AccountManagerServiceTestFixtures.CALLER_PACKAGE}; + when(mMockPackageManager.getPackagesForUid(anyInt())).thenReturn(list); + Account[] accounts = mAms.getAccountsAsUser(null, + UserHandle.getCallingUserId(), mContext.getOpPackageName()); + assertEquals(0, accounts.length); + } + + @SmallTest public void testPasswords() throws Exception { unlockSystemUser(); From 0c607e70982beb9ad4b2b7e6b459f596a620524b Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Tue, 28 Jun 2022 15:43:33 -0400 Subject: [PATCH 09/16] Enforce zen rule limit on a package level. This means that a single package with multiple different condition providers or configuration activities will correctly have all of their zen rules associated with the same package rather than each owner/activity having their rules counted separately. Bug: 235823407 Test: ZenModeHelperTest Change-Id: I35daf9a24f546ae25a78a2d841be39072cdc5641 Merged-In: I35daf9a24f546ae25a78a2d841be39072cdc5641 (cherry picked from commit f4e69394eee569ac3ec5748094b9ce88a91c278c) (cherry picked from commit aa14df0e501e7c0642bde6c89db72c017d2de0d4) Merged-In: I35daf9a24f546ae25a78a2d841be39072cdc5641 --- .../server/notification/ZenModeHelper.java | 20 ++++++++++- .../notification/ZenModeHelperTest.java | 35 +++++++++++++++++-- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/notification/ZenModeHelper.java b/services/core/java/com/android/server/notification/ZenModeHelper.java index 9e0c97502c4fb..2b00ad7c5cd76 100644 --- a/services/core/java/com/android/server/notification/ZenModeHelper.java +++ b/services/core/java/com/android/server/notification/ZenModeHelper.java @@ -330,7 +330,8 @@ public class ZenModeHelper { int newRuleInstanceCount = getCurrentInstanceCount(automaticZenRule.getOwner()) + getCurrentInstanceCount(automaticZenRule.getConfigurationActivity()) + 1; - if (newRuleInstanceCount > RULE_LIMIT_PER_PACKAGE + int newPackageRuleCount = getPackageRuleCount(pkg) + 1; + if (newPackageRuleCount > RULE_LIMIT_PER_PACKAGE || (ruleInstanceLimit > 0 && ruleInstanceLimit < newRuleInstanceCount)) { throw new IllegalArgumentException("Rule instance limit exceeded"); } @@ -511,6 +512,23 @@ public class ZenModeHelper { return count; } + // Equivalent method to getCurrentInstanceCount, but for all rules associated with a specific + // package rather than a condition provider service or activity. + private int getPackageRuleCount(String pkg) { + if (pkg == null) { + return 0; + } + int count = 0; + synchronized (mConfig) { + for (ZenRule rule : mConfig.automaticRules.values()) { + if (pkg.equals(rule.getPkg())) { + count++; + } + } + } + return count; + } + public boolean canManageAutomaticZenRule(ZenRule rule) { final int callingUid = Binder.getCallingUid(); if (callingUid == 0 || callingUid == Process.SYSTEM_UID) { diff --git a/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java index fd1536c5c0f10..4550b56f6fd0a 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java @@ -1622,7 +1622,9 @@ public class ZenModeHelperTest extends UiServiceTestCase { ZenModeConfig.toScheduleConditionId(si), new ZenPolicy.Builder().build(), NotificationManager.INTERRUPTION_FILTER_PRIORITY, true); - String id = mZenModeHelperSpy.addAutomaticZenRule("android", zenRule, "test"); + // We need the package name to be something that's not "android" so there aren't any + // existing rules under that package. + String id = mZenModeHelperSpy.addAutomaticZenRule("pkgname", zenRule, "test"); assertNotNull(id); } try { @@ -1632,12 +1634,41 @@ public class ZenModeHelperTest extends UiServiceTestCase { ZenModeConfig.toScheduleConditionId(new ScheduleInfo()), new ZenPolicy.Builder().build(), NotificationManager.INTERRUPTION_FILTER_PRIORITY, true); - String id = mZenModeHelperSpy.addAutomaticZenRule("android", zenRule, "test"); + String id = mZenModeHelperSpy.addAutomaticZenRule("pkgname", zenRule, "test"); fail("allowed too many rules to be created"); } catch (IllegalArgumentException e) { // yay } + } + @Test + public void testAddAutomaticZenRule_beyondSystemLimit_differentComponents() { + // Make sure the system limit is enforced per-package even with different component provider + // names. + for (int i = 0; i < RULE_LIMIT_PER_PACKAGE; i++) { + ScheduleInfo si = new ScheduleInfo(); + si.startHour = i; + AutomaticZenRule zenRule = new AutomaticZenRule("name" + i, + null, + new ComponentName("android", "ScheduleConditionProvider" + i), + ZenModeConfig.toScheduleConditionId(si), + new ZenPolicy.Builder().build(), + NotificationManager.INTERRUPTION_FILTER_PRIORITY, true); + String id = mZenModeHelperSpy.addAutomaticZenRule("pkgname", zenRule, "test"); + assertNotNull(id); + } + try { + AutomaticZenRule zenRule = new AutomaticZenRule("name", + null, + new ComponentName("android", "ScheduleConditionProviderFinal"), + ZenModeConfig.toScheduleConditionId(new ScheduleInfo()), + new ZenPolicy.Builder().build(), + NotificationManager.INTERRUPTION_FILTER_PRIORITY, true); + String id = mZenModeHelperSpy.addAutomaticZenRule("pkgname", zenRule, "test"); + fail("allowed too many rules to be created"); + } catch (IllegalArgumentException e) { + // yay + } } @Test From 64795ff0cf1e6ccdd16edfdece744e97b9b3015f Mon Sep 17 00:00:00 2001 From: shaoweishen Date: Mon, 8 Aug 2022 08:57:47 +0000 Subject: [PATCH 10/16] [Output Swithcer] Fix volume control issue for Group Bug: 241448326 Test: verified on device, atest MediaOutputAdapterTest Change-Id: I5d2ab2e1a6cb8341871305a729d8584ca15c4dcc (cherry picked from commit 9673316c56288ac8c5b055f1124f9f51cdb5aba6) (cherry picked from commit 2e1bb637d29e4e98c160cfdecfab0c41fb39ac1f) Merged-In: I5d2ab2e1a6cb8341871305a729d8584ca15c4dcc --- .../media/dialog/MediaOutputAdapter.java | 1 + .../media/dialog/MediaOutputBaseAdapter.java | 9 ++++++- .../media/dialog/MediaOutputMetricLogger.java | 8 ++++++ .../media/dialog/MediaOutputAdapterTest.java | 26 +++++++++++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputAdapter.java b/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputAdapter.java index f2f275323d58d..1ab0b5e263d1c 100644 --- a/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputAdapter.java +++ b/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputAdapter.java @@ -269,6 +269,7 @@ public class MediaOutputAdapter extends MediaOutputBaseAdapter { } private void onGroupActionTriggered(boolean isChecked, MediaDevice device) { + disableSeekBar(); if (isChecked && isDeviceIncluded(mController.getSelectableMediaDevice(), device)) { mController.addDeviceToPlayMedia(device); } else if (!isChecked && isDeviceIncluded(mController.getDeselectableMediaDevice(), diff --git a/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputBaseAdapter.java b/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputBaseAdapter.java index bec67397a926e..3b4ca48046eb0 100644 --- a/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputBaseAdapter.java +++ b/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputBaseAdapter.java @@ -273,6 +273,8 @@ public abstract class MediaOutputBaseAdapter extends void initSeekbar(MediaDevice device, boolean isCurrentSeekbarInvisible) { if (!mController.isVolumeControlEnabled(device)) { disableSeekBar(); + } else { + enableSeekBar(); } mSeekBar.setMaxVolume(device.getMaxVolume()); final int currentVolume = device.getCurrentVolume(); @@ -417,11 +419,16 @@ public abstract class MediaOutputBaseAdapter extends return drawable; } - private void disableSeekBar() { + protected void disableSeekBar() { mSeekBar.setEnabled(false); mSeekBar.setOnTouchListener((v, event) -> true); } + private void enableSeekBar() { + mSeekBar.setEnabled(true); + mSeekBar.setOnTouchListener((v, event) -> false); + } + protected void setUpDeviceIcon(MediaDevice device) { ThreadUtils.postOnBackgroundThread(() -> { Icon icon = mController.getDeviceIconCompat(device).toIcon(mContext); diff --git a/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputMetricLogger.java b/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputMetricLogger.java index 5d7af522176a0..6fe06e0855566 100644 --- a/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputMetricLogger.java +++ b/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputMetricLogger.java @@ -194,6 +194,11 @@ public class MediaOutputMetricLogger { } private int getLoggingDeviceType(MediaDevice device, boolean isSourceDevice) { + if (device == null) { + return isSourceDevice + ? SysUiStatsLog.MEDIA_OUTPUT_OP_SWITCH_REPORTED__SOURCE__UNKNOWN_TYPE + : SysUiStatsLog.MEDIA_OUTPUT_OP_SWITCH_REPORTED__TARGET__UNKNOWN_TYPE; + } switch (device.getDeviceType()) { case MediaDevice.MediaDeviceType.TYPE_PHONE_DEVICE: return isSourceDevice @@ -229,6 +234,9 @@ public class MediaOutputMetricLogger { } private int getInteractionDeviceType(MediaDevice device) { + if (device == null) { + return SysUiStatsLog.MEDIA_OUTPUT_OP_INTERACTION_REPORTED__TARGET__UNKNOWN_TYPE; + } switch (device.getDeviceType()) { case MediaDevice.MediaDeviceType.TYPE_PHONE_DEVICE: return SysUiStatsLog.MEDIA_OUTPUT_OP_INTERACTION_REPORTED__TARGET__BUILTIN_SPEAKER; diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/dialog/MediaOutputAdapterTest.java b/packages/SystemUI/tests/src/com/android/systemui/media/dialog/MediaOutputAdapterTest.java index 59475cf0cb90c..c4cb8339c4d60 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/dialog/MediaOutputAdapterTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/media/dialog/MediaOutputAdapterTest.java @@ -274,4 +274,30 @@ public class MediaOutputAdapterTest extends SysuiTestCase { verify(mMediaOutputController).connectDevice(mMediaDevice2); } + + @Test + public void onItemClick_onGroupActionTriggered_verifySeekbarDisabled() { + when(mMediaOutputController.getSelectedMediaDevice()).thenReturn(mMediaDevices); + List selectableDevices = new ArrayList<>(); + selectableDevices.add(mMediaDevice1); + when(mMediaOutputController.getSelectableMediaDevice()).thenReturn(selectableDevices); + when(mMediaOutputController.hasAdjustVolumeUserRestriction()).thenReturn(true); + mMediaOutputAdapter.onBindViewHolder(mViewHolder, 0); + + mViewHolder.mContainerLayout.performClick(); + + assertThat(mViewHolder.mSeekBar.isEnabled()).isFalse(); + } + + @Test + public void onBindViewHolder_volumeControlChangeToEnabled_enableSeekbarAgain() { + when(mMediaOutputController.isVolumeControlEnabled(mMediaDevice1)).thenReturn(false); + mMediaOutputAdapter.onBindViewHolder(mViewHolder, 0); + assertThat(mViewHolder.mSeekBar.isEnabled()).isFalse(); + + when(mMediaOutputController.isVolumeControlEnabled(mMediaDevice1)).thenReturn(true); + mMediaOutputAdapter.onBindViewHolder(mViewHolder, 0); + + assertThat(mViewHolder.mSeekBar.isEnabled()).isTrue(); + } } From 8c1f443b554e1b78801ecffdbbe0e5274c9510d7 Mon Sep 17 00:00:00 2001 From: shaoweishen Date: Fri, 12 Aug 2022 01:32:24 +0000 Subject: [PATCH 11/16] [Output Switcher] Add platform config for Output Switcher add config indicate Output Switcher's version for supporting different feature on remote devices operation. Test: build pass Bug: 241448326 Change-Id: Ic09daff5ec63d1124a518ff3edc6bb059b150b80 (cherry picked from commit 56473a5946a430b6ab0be2f16a03f2386d91e4fb) Merged-In: Ic09daff5ec63d1124a518ff3edc6bb059b150b80 --- core/res/res/values/config.xml | 3 +++ core/res/res/values/symbols.xml | 2 ++ 2 files changed, 5 insertions(+) diff --git a/core/res/res/values/config.xml b/core/res/res/values/config.xml index 31229e97024fd..6995cc3b94ea5 100644 --- a/core/res/res/values/config.xml +++ b/core/res/res/values/config.xml @@ -2034,6 +2034,9 @@ on grouped devices. --> true + + 1 + false diff --git a/core/res/res/values/symbols.xml b/core/res/res/values/symbols.xml index 4e7e20ca10b0a..fbb2e4afcda4c 100644 --- a/core/res/res/values/symbols.xml +++ b/core/res/res/values/symbols.xml @@ -4679,6 +4679,8 @@ + + From 1b9dc28da9496007bff3a93fc62a2069dd152779 Mon Sep 17 00:00:00 2001 From: Jernej Virag Date: Fri, 12 Aug 2022 18:06:23 +0200 Subject: [PATCH 12/16] Prevent loading of Icon resources from the wrong package If an Icon carries package designator, LocalImageResolver has now ignored it. This could lead to loading of a wrong icon in cases where the ID collided with the same ID in android package. This is fixes that corner-case. Bug: 241066484 Test: atest LocalImageResolverTest - the new test checks for pkg correctness and fails on devices without this cl manual test: bluejay device with known, reproducible resource collision in media player notification. In reproducible case, icon appears as a broken block. After applying this patch, small icon in media notification loads correctly for the collision case. Change-Id: Ic79c0d4acb1d347a24282972e009a5a079ce6c21 (cherry picked from commit 32f176e4705694c9d0272eba22887cf6d4f7b783) Merged-In: Ic79c0d4acb1d347a24282972e009a5a079ce6c21 --- .../com/android/internal/widget/LocalImageResolver.java | 7 +++++++ .../android/internal/widget/LocalImageResolverTest.java | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/core/java/com/android/internal/widget/LocalImageResolver.java b/core/java/com/android/internal/widget/LocalImageResolver.java index b866723954b5b..b11ea2961c179 100644 --- a/core/java/com/android/internal/widget/LocalImageResolver.java +++ b/core/java/com/android/internal/widget/LocalImageResolver.java @@ -25,6 +25,7 @@ import android.graphics.ImageDecoder; import android.graphics.drawable.Drawable; import android.graphics.drawable.Icon; import android.net.Uri; +import android.text.TextUtils; import android.util.Log; import android.util.Size; @@ -108,6 +109,12 @@ public class LocalImageResolver { } break; case Icon.TYPE_RESOURCE: + if (!(TextUtils.isEmpty(icon.getResPackage()) + || context.getPackageName().equals(icon.getResPackage()))) { + // We can't properly resolve icons from other packages here, so fall back. + return icon.loadDrawable(context); + } + Drawable result = resolveImage(icon.getResId(), context, maxWidth, maxHeight); if (result != null) { return tintDrawable(icon, result); diff --git a/core/tests/coretests/src/com/android/internal/widget/LocalImageResolverTest.java b/core/tests/coretests/src/com/android/internal/widget/LocalImageResolverTest.java index c63d18bfa5315..0cee526651a67 100644 --- a/core/tests/coretests/src/com/android/internal/widget/LocalImageResolverTest.java +++ b/core/tests/coretests/src/com/android/internal/widget/LocalImageResolverTest.java @@ -270,4 +270,13 @@ public class LocalImageResolverTest { assertThat(bd.getBitmap().getHeight()).isEqualTo(originalHeight); } + + @Test + public void resolveImage_iconWithOtherPackageResource_usesPackageContextDefinition() + throws IOException { + Icon icon = Icon.createWithResource("this_is_invalid", R.drawable.test32x24); + Drawable d = LocalImageResolver.resolveImage(icon, mContext); + // This drawable must not be loaded - if it was, the code ignored the package specification. + assertThat(d).isNull(); + } } From ccaebb4f641e0d42b7c0c099769dd23a3f02b9fe Mon Sep 17 00:00:00 2001 From: wilsonshih Date: Mon, 23 May 2022 12:36:26 +0800 Subject: [PATCH 13/16] Do not create starting window if there is no task. Do not add starting window on the activity if no task exists in the activity. Also add an error log for the activity without task. Bug: 233153876 Bug: 240032099 Test: atest SplashscreenTests ActivityRecordTests Change-Id: Iea264cd8b605443c695165c1136d56b86ffd4eb1 Merged-In: Iea264cd8b605443c695165c1136d56b86ffd4eb1 (cherry picked from commit 24697fe3fdf92cac03b553892a3e1d17ae30c8a1) (cherry picked from commit f5909156007fb7976c34ec187a68792d92a6edc2) Merged-In: Iea264cd8b605443c695165c1136d56b86ffd4eb1 --- .../com/android/server/wm/StartingSurfaceController.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/services/core/java/com/android/server/wm/StartingSurfaceController.java b/services/core/java/com/android/server/wm/StartingSurfaceController.java index f83173bd46c01..0bb773ae5e410 100644 --- a/services/core/java/com/android/server/wm/StartingSurfaceController.java +++ b/services/core/java/com/android/server/wm/StartingSurfaceController.java @@ -220,6 +220,11 @@ public class StartingSurfaceController { // Attempt to add starting window from the top-most activity. for (int i = mDeferringAddStartActivities.size() - 1; i >= 0; --i) { final DeferringStartingWindowRecord next = mDeferringAddStartActivities.get(i); + if (next.mDeferring.getTask() == null) { + Slog.e(TAG, "No task exists: " + next.mDeferring.shortComponentName + + " parent: " + next.mDeferring.getParent()); + continue; + } next.mDeferring.showStartingWindow(next.mPrev, mInitNewTask, mInitTaskSwitch, mInitProcessRunning, true /* startActivity */, next.mSource, topOptions); // If one succeeds, it is done. From 22ab60b4ed3804e9a386bf4b81197f99d5f058fc Mon Sep 17 00:00:00 2001 From: Tim Murray Date: Fri, 22 Jul 2022 14:44:00 -0700 Subject: [PATCH 14/16] RemoteViews: fix BitmapCache after immutable Bitmap switch Test: widgets appear correctly Bug: 237256275 Change-Id: I864ac1ba8ac771c326dc215ae4e712ceb0b6057b (cherry picked from commit 4f359bd7655ba6d4f647bb62b877b721f25bbeb5) Merged-In: I864ac1ba8ac771c326dc215ae4e712ceb0b6057b --- core/java/android/widget/RemoteViews.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/core/java/android/widget/RemoteViews.java b/core/java/android/widget/RemoteViews.java index 2879cd888d2d8..bc7e31fac222a 100644 --- a/core/java/android/widget/RemoteViews.java +++ b/core/java/android/widget/RemoteViews.java @@ -1598,7 +1598,13 @@ public class RemoteViews implements Parcelable, Filter { public BitmapCache(Parcel source) { mBitmaps = source.createTypedArrayList(Bitmap.CREATOR); - mBitmapHashes = source.readSparseIntArray(); + mBitmapHashes = new SparseIntArray(); + for (int i = 0; i < mBitmaps.size(); i++) { + Bitmap b = mBitmaps.get(i); + if (b != null) { + mBitmapHashes.put(b.hashCode(), i); + } + } } public int getBitmapId(Bitmap b) { @@ -1614,7 +1620,7 @@ public class RemoteViews implements Parcelable, Filter { b = b.asShared(); } mBitmaps.add(b); - mBitmapHashes.put(mBitmaps.size() - 1, hash); + mBitmapHashes.put(hash, mBitmaps.size() - 1); mBitmapMemory = -1; return (mBitmaps.size() - 1); } @@ -1631,7 +1637,6 @@ public class RemoteViews implements Parcelable, Filter { public void writeBitmapsToParcel(Parcel dest, int flags) { dest.writeTypedList(mBitmaps, flags); - dest.writeSparseIntArray(mBitmapHashes); } public int getBitmapMemory() { From c6f280af4d647c4066f4ef33c55c62d4285fb2da Mon Sep 17 00:00:00 2001 From: lucaslin Date: Mon, 25 Jul 2022 15:07:52 +0800 Subject: [PATCH 15/16] Skip sending VpnManager events for Settings VPNs This change early-exits from the sendEventToVpnManagerApp() method if the profile was not a VPN app. Otherwise the sendEventToVpnManagerApp() will call getRedactedNetworkCapabilities() which will call AppOpsManager#checkPackage() eventually. And AppOpsManager#checkPackage() will check if the given package is the same as the given uid. In this case, VPN sends "[Legacy VPN]" as the package and sends 1000 as the uid, but there is no package named "[Legacy VPN], so the SecurityException is thrown. Bug: 236315805 Test: atest FrameworksNetTests:VpnTest Change-Id: I486398111106b1a9551fb29f92ba7b4fe85f68c1 Merged-In: I486398111106b1a9551fb29f92ba7b4fe85f68c1 Merged-In: I228f62a5e09017dbf985a614f2e42434238a220c (cherry picked from commit a7ec0cbe45dd75301b19290174ecf2da6421cf97) Merged-In: I486398111106b1a9551fb29f92ba7b4fe85f68c1 --- .../java/com/android/server/connectivity/Vpn.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index 77d3392da9931..15aa07f406419 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -2944,7 +2944,7 @@ public class Vpn { // All the above failures are configuration errors, and are terminal // TODO(b/230548427): Remove SDK check once VPN related stuff are // decoupled from ConnectivityServiceTest. - if (SdkLevel.isAtLeastT()) { + if (SdkLevel.isAtLeastT() && isVpnApp(mPackage)) { sendEventToVpnManagerApp(VpnManager.CATEGORY_EVENT_IKE_ERROR, VpnManager.ERROR_CLASS_NOT_RECOVERABLE, ikeException.getErrorType(), @@ -2962,7 +2962,7 @@ public class Vpn { // All the above failures are configuration errors, and are terminal // TODO(b/230548427): Remove SDK check once VPN related stuff are // decoupled from ConnectivityServiceTest. - if (SdkLevel.isAtLeastT()) { + if (SdkLevel.isAtLeastT() && isVpnApp(mPackage)) { sendEventToVpnManagerApp(VpnManager.CATEGORY_EVENT_IKE_ERROR, VpnManager.ERROR_CLASS_RECOVERABLE, ikeException.getErrorType(), @@ -2981,7 +2981,7 @@ public class Vpn { } else if (exception instanceof IkeNetworkLostException) { // TODO(b/230548427): Remove SDK check once VPN related stuff are // decoupled from ConnectivityServiceTest. - if (SdkLevel.isAtLeastT()) { + if (SdkLevel.isAtLeastT() && isVpnApp(mPackage)) { sendEventToVpnManagerApp(VpnManager.CATEGORY_EVENT_NETWORK_ERROR, VpnManager.ERROR_CLASS_RECOVERABLE, VpnManager.ERROR_CODE_NETWORK_LOST, @@ -2996,7 +2996,7 @@ public class Vpn { if (exception.getCause() instanceof UnknownHostException) { // TODO(b/230548427): Remove SDK check once VPN related stuff are // decoupled from ConnectivityServiceTest. - if (SdkLevel.isAtLeastT()) { + if (SdkLevel.isAtLeastT() && isVpnApp(mPackage)) { sendEventToVpnManagerApp(VpnManager.CATEGORY_EVENT_NETWORK_ERROR, VpnManager.ERROR_CLASS_RECOVERABLE, VpnManager.ERROR_CODE_NETWORK_UNKNOWN_HOST, @@ -3010,7 +3010,7 @@ public class Vpn { } else if (exception.getCause() instanceof IkeTimeoutException) { // TODO(b/230548427): Remove SDK check once VPN related stuff are // decoupled from ConnectivityServiceTest. - if (SdkLevel.isAtLeastT()) { + if (SdkLevel.isAtLeastT() && isVpnApp(mPackage)) { sendEventToVpnManagerApp(VpnManager.CATEGORY_EVENT_NETWORK_ERROR, VpnManager.ERROR_CLASS_RECOVERABLE, VpnManager.ERROR_CODE_NETWORK_PROTOCOL_TIMEOUT, @@ -3024,7 +3024,7 @@ public class Vpn { } else if (exception.getCause() instanceof IOException) { // TODO(b/230548427): Remove SDK check once VPN related stuff are // decoupled from ConnectivityServiceTest. - if (SdkLevel.isAtLeastT()) { + if (SdkLevel.isAtLeastT() && isVpnApp(mPackage)) { sendEventToVpnManagerApp(VpnManager.CATEGORY_EVENT_NETWORK_ERROR, VpnManager.ERROR_CLASS_RECOVERABLE, VpnManager.ERROR_CODE_NETWORK_IO, From 82bcb2a43449caedc18a300f2dd2a7bc81541602 Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Thu, 21 Jul 2022 11:50:22 -0400 Subject: [PATCH 16/16] Fix race condition in QSTileHost QSTileHost had three problems that caused a race condition: * Calls to add/remove tiles could come and be executed in any thread. * Different paths to add/remove tiles used different ground truths on what tiles were currently active. * The list of tiles contained in mTileSpecs was not necessarily the same as the visible tiles. This caused the following problem: * If two tiles (usually CustomTile) were removed roughly at the same time, only one of them would actually be removed from the Setting, as both of them would race to remove from the same list. This caused... * There would be leftover tiles that cannot connect to a TileService and therefore show empty UI. The solution is as follows: * Make sure that all operations that modify the tiles are performed in the main thread. * Make sure that all operations that modify the tiles use the same truth for the current state. * Make sure that mTileSpecs always reflects the setting at the end of onTuningChanged. This means that tiles that are destroyed or removed will never be in this list. Test: atest com.android.systemui.qs Test: manual, start Guest user and notice that there are no blank tiles. Fixes: 235561921 Change-Id: I6ffbb7510e6854f18bc427d1ae1e782918c799dc Merged-In: I6ffbb7510e6854f18bc427d1ae1e782918c799dc (cherry picked from commit 7f2bb15f4e40c0e1d1d0b2308444f4f9f449bd3f) (cherry picked from commit 2d3949485591a31a90e7abec5ac7a3dc8597072a) Merged-In: I6ffbb7510e6854f18bc427d1ae1e782918c799dc --- .../com/android/systemui/qs/QSTileHost.java | 132 +++++++---- .../systemui/qs/customize/TileAdapter.java | 4 +- .../qs/external/TileServiceManager.java | 2 +- .../statusbar/phone/AutoTileManager.java | 2 +- .../CentralSurfacesCommandQueueCallbacks.java | 2 +- .../android/systemui/qs/QSFragmentTest.java | 40 ---- .../android/systemui/qs/QSTileHostTest.java | 205 +++++++++++++----- .../qs/customize/TileAdapterTest.java | 2 +- .../qs/external/TileServicesTest.java | 7 +- .../statusbar/phone/AutoTileManagerTest.java | 2 +- 10 files changed, 246 insertions(+), 152 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java b/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java index fbdabc74aba48..fcd9e10089bcf 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java +++ b/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java @@ -19,8 +19,6 @@ import android.content.Context; import android.content.Intent; import android.content.res.Resources; import android.os.Build; -import android.os.Handler; -import android.os.Looper; import android.os.UserHandle; import android.os.UserManager; import android.provider.Settings.Secure; @@ -28,6 +26,7 @@ import android.text.TextUtils; import android.util.ArraySet; import android.util.Log; +import androidx.annotation.MainThread; import androidx.annotation.Nullable; import com.android.internal.logging.InstanceId; @@ -35,9 +34,7 @@ import com.android.internal.logging.InstanceIdSequence; import com.android.internal.logging.UiEventLogger; import com.android.systemui.Dumpable; import com.android.systemui.R; -import com.android.systemui.broadcast.BroadcastDispatcher; import com.android.systemui.dagger.SysUISingleton; -import com.android.systemui.dagger.qualifiers.Background; import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.dump.DumpManager; import com.android.systemui.plugins.PluginListener; @@ -68,12 +65,20 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.concurrent.Executor; import java.util.function.Predicate; import javax.inject.Inject; import javax.inject.Provider; -/** Platform implementation of the quick settings tile host **/ +/** Platform implementation of the quick settings tile host + * + * This class keeps track of the set of current tiles and is the in memory source of truth + * (ground truth is kept in {@link Secure#QS_TILES}). When the ground truth changes, + * {@link #onTuningChanged} will be called and the tiles will be re-created as needed. + * + * This class also provides the interface for adding/removing/changing tiles. + */ @SysUISingleton public class QSTileHost implements QSHost, Tunable, PluginListener, Dumpable { private static final String TAG = "QSTileHost"; @@ -89,11 +94,11 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D private final TunerService mTunerService; private final PluginManager mPluginManager; private final DumpManager mDumpManager; - private final BroadcastDispatcher mBroadcastDispatcher; private final QSLogger mQSLogger; private final UiEventLogger mUiEventLogger; private final InstanceIdSequence mInstanceIdSequence; private final CustomTileStatePersister mCustomTileStatePersister; + private final Executor mMainExecutor; private final List mCallbacks = new ArrayList<>(); @Nullable @@ -113,13 +118,11 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D public QSTileHost(Context context, StatusBarIconController iconController, QSFactory defaultFactory, - @Main Handler mainHandler, - @Background Looper bgLooper, + @Main Executor mainExecutor, PluginManager pluginManager, TunerService tunerService, Provider autoTiles, DumpManager dumpManager, - BroadcastDispatcher broadcastDispatcher, Optional centralSurfacesOptional, QSLogger qsLogger, UiEventLogger uiEventLogger, @@ -137,7 +140,7 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D mDumpManager = dumpManager; mQSLogger = qsLogger; mUiEventLogger = uiEventLogger; - mBroadcastDispatcher = broadcastDispatcher; + mMainExecutor = mainExecutor; mTileServiceRequestController = tileServiceRequestControllerBuilder.create(this); mTileLifeCycleManagerFactory = tileLifecycleManagerFactory; @@ -151,7 +154,7 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D mSecureSettings = secureSettings; mCustomTileStatePersister = customTileStatePersister; - mainHandler.post(() -> { + mainExecutor.execute(() -> { // This is technically a hack to avoid circular dependency of // QSTileHost -> XXXTile -> QSTileHost. Posting ensures creation // finishes before creating any tiles. @@ -258,6 +261,33 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D return mTileSpecs.indexOf(spec); } + /** + * Whenever the Secure Setting keeping track of the current tiles changes (or upon start) this + * will be called with the new value of the setting. + * + * This method will do the following: + *
    + *
  1. Destroy any existing tile that's not one of the current tiles (in the setting)
    2. + *
  2. Create new tiles for those that don't already exist. If this tiles end up being + * not available, they'll also be destroyed.
    3. + *
  3. Save the resolved list of tiles (current tiles that are available) into the setting. + * This means that after this call ends, the tiles in the Setting, {@link #mTileSpecs}, + * and visible tiles ({@link #mTiles}) must match. + *
    4. + *
+ * + * Additionally, if the user has changed, it'll do the following: + *
    + *
  • Change the user for SystemUI tiles: {@link QSTile#userSwitch}.
    • + *
  • Destroy any {@link CustomTile} and recreate it for the new user.
    • + *
+ * + * This happens in main thread as {@link com.android.systemui.tuner.TunerServiceImpl} dispatches + * in main thread. + * + * @see QSTile#isAvailable + */ + @MainThread @Override public void onTuningChanged(String key, String newValue) { if (!TILES_SETTING.equals(key)) { @@ -330,34 +360,44 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D mCurrentUser = currentUser; List currentSpecs = new ArrayList<>(mTileSpecs); mTileSpecs.clear(); - mTileSpecs.addAll(tileSpecs); + mTileSpecs.addAll(newTiles.keySet()); // Only add the valid (available) tiles. mTiles.clear(); mTiles.putAll(newTiles); if (newTiles.isEmpty() && !tileSpecs.isEmpty()) { // If we didn't manage to create any tiles, set it to empty (default) Log.d(TAG, "No valid tiles on tuning changed. Setting to default."); - changeTiles(currentSpecs, loadTileSpecs(mContext, "")); + changeTilesByUser(currentSpecs, loadTileSpecs(mContext, "")); } else { + String resolvedTiles = TextUtils.join(",", mTileSpecs); + if (!resolvedTiles.equals(newValue)) { + // If the resolved tiles (those we actually ended up with) are different than + // the ones that are in the setting, update the Setting. + saveTilesToSettings(mTileSpecs); + } for (int i = 0; i < mCallbacks.size(); i++) { mCallbacks.get(i).onTilesChanged(); } } } + /** + * Only use with [CustomTile] if the tile doesn't exist anymore (and therefore doesn't need + * its lifecycle terminated). + */ @Override public void removeTile(String spec) { - changeTileSpecs(tileSpecs-> tileSpecs.remove(spec)); + mMainExecutor.execute(() -> changeTileSpecs(tileSpecs-> tileSpecs.remove(spec))); } /** * Remove many tiles at once. * - * It will only save to settings once (as opposed to {@link QSTileHost#removeTile} called + * It will only save to settings once (as opposed to {@link QSTileHost#removeTileByUser} called * multiple times). */ @Override public void removeTiles(Collection specs) { - changeTileSpecs(tileSpecs -> tileSpecs.removeAll(specs)); + mMainExecutor.execute(() -> changeTileSpecs(tileSpecs -> tileSpecs.removeAll(specs))); } @Override @@ -381,27 +421,30 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D * @param requestPosition -1 for end, 0 for beginning, or X for insertion at position X */ public void addTile(String spec, int requestPosition) { - if (spec.equals("work")) Log.wtfStack(TAG, "Adding work tile"); - changeTileSpecs(tileSpecs -> { - if (tileSpecs.contains(spec)) return false; + mMainExecutor.execute(() -> + changeTileSpecs(tileSpecs -> { + if (tileSpecs.contains(spec)) return false; - int size = tileSpecs.size(); - if (requestPosition == POSITION_AT_END || requestPosition >= size) { - tileSpecs.add(spec); - } else { - tileSpecs.add(requestPosition, spec); - } - return true; - }); + int size = tileSpecs.size(); + if (requestPosition == POSITION_AT_END || requestPosition >= size) { + tileSpecs.add(spec); + } else { + tileSpecs.add(requestPosition, spec); + } + return true; + }) + ); } - void saveTilesToSettings(List tileSpecs) { - if (tileSpecs.contains("work")) Log.wtfStack(TAG, "Saving work tile"); + + @MainThread + private void saveTilesToSettings(List tileSpecs) { mSecureSettings.putStringForUser(TILES_SETTING, TextUtils.join(",", tileSpecs), null /* tag */, false /* default */, mCurrentUser, true /* overrideable by restore */); } + @MainThread private void changeTileSpecs(Predicate> changeFunction) { final String setting = mSecureSettings.getStringForUser(TILES_SETTING, mCurrentUser); final List tileSpecs = loadTileSpecs(mContext, setting); @@ -421,29 +464,32 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D */ public void addTile(ComponentName tile, boolean end) { String spec = CustomTile.toSpec(tile); - if (!mTileSpecs.contains(spec)) { - List newSpecs = new ArrayList<>(mTileSpecs); - if (end) { - newSpecs.add(spec); - } else { - newSpecs.add(0, spec); - } - changeTiles(mTileSpecs, newSpecs); - } + addTile(spec, end ? POSITION_AT_END : 0); } - public void removeTile(ComponentName tile) { - List newSpecs = new ArrayList<>(mTileSpecs); - newSpecs.remove(CustomTile.toSpec(tile)); - changeTiles(mTileSpecs, newSpecs); + /** + * This will call through {@link #changeTilesByUser}. It should only be used when a tile is + * removed by a user action like {@code adb}. + */ + public void removeTileByUser(ComponentName tile) { + mMainExecutor.execute(() -> { + List newSpecs = new ArrayList<>(mTileSpecs); + if (newSpecs.remove(CustomTile.toSpec(tile))) { + changeTilesByUser(mTileSpecs, newSpecs); + } + }); } /** * Change the tiles triggered by the user editing. *

* This is not called on device start, or on user change. + * + * {@link android.service.quicksettings.TileService#onTileRemoved} will be called for tiles + * that are removed. */ - public void changeTiles(List previousTiles, List newTiles) { + @MainThread + public void changeTilesByUser(List previousTiles, List newTiles) { final List copy = new ArrayList<>(previousTiles); final int NP = copy.size(); for (int i = 0; i < NP; i++) { diff --git a/packages/SystemUI/src/com/android/systemui/qs/customize/TileAdapter.java b/packages/SystemUI/src/com/android/systemui/qs/customize/TileAdapter.java index e52bfbd67275f..d84b12c714bdd 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/customize/TileAdapter.java +++ b/packages/SystemUI/src/com/android/systemui/qs/customize/TileAdapter.java @@ -182,7 +182,7 @@ public class TileAdapter extends RecyclerView.Adapter implements TileSta for (int i = 1; i < mTiles.size() && mTiles.get(i) != null; i++) { newSpecs.add(mTiles.get(i).spec); } - host.changeTiles(mCurrentSpecs, newSpecs); + host.changeTilesByUser(mCurrentSpecs, newSpecs); mCurrentSpecs = newSpecs; } @@ -200,7 +200,7 @@ public class TileAdapter extends RecyclerView.Adapter implements TileSta /** */ public void resetTileSpecs(List specs) { // Notify the host so the tiles get removed callbacks. - mHost.changeTiles(mCurrentSpecs, specs); + mHost.changeTilesByUser(mCurrentSpecs, specs); setTileSpecs(specs); } diff --git a/packages/SystemUI/src/com/android/systemui/qs/external/TileServiceManager.java b/packages/SystemUI/src/com/android/systemui/qs/external/TileServiceManager.java index bf565a8c52e05..cfc57db2eeb88 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/external/TileServiceManager.java +++ b/packages/SystemUI/src/com/android/systemui/qs/external/TileServiceManager.java @@ -289,7 +289,7 @@ public class TileServiceManager { } } - mServices.getHost().removeTile(component); + mServices.getHost().removeTile(CustomTile.toSpec(component)); } }; } 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 8782be50794f7..9070eadd9944f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java @@ -428,7 +428,7 @@ public class AutoTileManager implements UserAwareController { if (isSafetyCenterEnabled && !mAutoTracker.isAdded(mSafetySpec)) { initSafetyTile(); } else if (!isSafetyCenterEnabled && mAutoTracker.isAdded(mSafetySpec)) { - mHost.removeTile(CustomTile.getComponentFromSpec(mSafetySpec)); + mHost.removeTile(mSafetySpec); mHost.unmarkTileAsAutoAdded(mSafetySpec); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesCommandQueueCallbacks.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesCommandQueueCallbacks.java index 9060d5f67913a..ffd50ab5af377 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesCommandQueueCallbacks.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesCommandQueueCallbacks.java @@ -187,7 +187,7 @@ public class CentralSurfacesCommandQueueCallbacks implements CommandQueue.Callba public void remQsTile(ComponentName tile) { QSPanelController qsPanelController = mCentralSurfaces.getQSPanelController(); if (qsPanelController != null && qsPanelController.getHost() != null) { - qsPanelController.getHost().removeTile(tile); + qsPanelController.getHost().removeTileByUser(tile); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/QSFragmentTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/QSFragmentTest.java index 664af75a6529a..32c66d25d4aa6 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/QSFragmentTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QSFragmentTest.java @@ -32,8 +32,6 @@ import android.app.Fragment; import android.content.Context; import android.graphics.Rect; import android.os.Bundle; -import android.os.Handler; -import android.os.Looper; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper.RunWithLooper; import android.view.LayoutInflater; @@ -42,36 +40,23 @@ import android.view.ViewGroup; import androidx.test.filters.SmallTest; -import com.android.internal.logging.UiEventLogger; import com.android.keyguard.BouncerPanelExpansionCalculator; import com.android.systemui.R; import com.android.systemui.SysuiBaseFragmentTest; import com.android.systemui.animation.ShadeInterpolation; -import com.android.systemui.broadcast.BroadcastDispatcher; import com.android.systemui.dump.DumpManager; import com.android.systemui.media.MediaHost; import com.android.systemui.plugins.FalsingManager; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.qs.customize.QSCustomizerController; import com.android.systemui.qs.dagger.QSFragmentComponent; -import com.android.systemui.qs.external.CustomTileStatePersister; -import com.android.systemui.qs.external.TileLifecycleManager; import com.android.systemui.qs.external.TileServiceRequestController; -import com.android.systemui.qs.logging.QSLogger; -import com.android.systemui.qs.tileimpl.QSFactoryImpl; -import com.android.systemui.settings.UserTracker; -import com.android.systemui.shared.plugins.PluginManager; import com.android.systemui.statusbar.CommandQueue; import com.android.systemui.statusbar.StatusBarState; -import com.android.systemui.statusbar.phone.AutoTileManager; -import com.android.systemui.statusbar.phone.CentralSurfaces; import com.android.systemui.statusbar.phone.KeyguardBypassController; -import com.android.systemui.statusbar.phone.StatusBarIconController; import com.android.systemui.statusbar.policy.ConfigurationController; import com.android.systemui.statusbar.policy.RemoteInputQuickSettingsDisabler; -import com.android.systemui.tuner.TunerService; import com.android.systemui.util.animation.UniqueObjectHostView; -import com.android.systemui.util.settings.SecureSettings; import org.junit.Before; import org.junit.Test; @@ -79,8 +64,6 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import java.util.Optional; - @RunWith(AndroidTestingRunner.class) @RunWithLooper(setAsMainLooper = true) @SmallTest @@ -125,34 +108,11 @@ public class QSFragmentTest extends SysuiBaseFragmentTest { mFragments.dispatchResume(); processAllMessages(); - QSTileHost host = - new QSTileHost( - mContext, - mock(StatusBarIconController.class), - mock(QSFactoryImpl.class), - new Handler(), - Looper.myLooper(), - mock(PluginManager.class), - mock(TunerService.class), - () -> mock(AutoTileManager.class), - mock(DumpManager.class), - mock(BroadcastDispatcher.class), - Optional.of(mock(CentralSurfaces.class)), - mock(QSLogger.class), - mock(UiEventLogger.class), - mock(UserTracker.class), - mock(SecureSettings.class), - mock(CustomTileStatePersister.class), - mTileServiceRequestControllerBuilder, - mock(TileLifecycleManager.Factory.class)); - qs.setListening(true); processAllMessages(); qs.setListening(false); processAllMessages(); - host.destroy(); - processAllMessages(); } @Test 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 8cf3fe274848e..7dbc561fbfc14 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java @@ -32,12 +32,11 @@ import static org.mockito.Mockito.when; import android.content.ComponentName; import android.content.Context; import android.content.Intent; +import android.database.ContentObserver; import android.os.Handler; import android.os.Looper; import android.os.UserHandle; import android.testing.AndroidTestingRunner; -import android.testing.TestableLooper; -import android.testing.TestableLooper.RunWithLooper; import android.view.View; import androidx.annotation.Nullable; @@ -48,7 +47,6 @@ import com.android.internal.logging.UiEventLogger; import com.android.internal.util.CollectionUtils; import com.android.systemui.R; import com.android.systemui.SysuiTestCase; -import com.android.systemui.broadcast.BroadcastDispatcher; import com.android.systemui.classifier.FalsingManagerFake; import com.android.systemui.dump.DumpManager; import com.android.systemui.plugins.ActivityStarter; @@ -68,8 +66,10 @@ import com.android.systemui.statusbar.phone.AutoTileManager; import com.android.systemui.statusbar.phone.CentralSurfaces; import com.android.systemui.statusbar.phone.StatusBarIconController; import com.android.systemui.tuner.TunerService; +import com.android.systemui.util.concurrency.FakeExecutor; import com.android.systemui.util.settings.FakeSettings; import com.android.systemui.util.settings.SecureSettings; +import com.android.systemui.util.time.FakeSystemClock; import org.junit.Before; import org.junit.Test; @@ -81,18 +81,19 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.util.List; import java.util.Optional; +import java.util.concurrent.Executor; import javax.inject.Provider; @RunWith(AndroidTestingRunner.class) @SmallTest -@RunWithLooper(setAsMainLooper = true) public class QSTileHostTest extends SysuiTestCase { private static String MOCK_STATE_STRING = "MockState"; private static ComponentName CUSTOM_TILE = ComponentName.unflattenFromString("TEST_PKG/.TEST_CLS"); private static final String CUSTOM_TILE_SPEC = CustomTile.toSpec(CUSTOM_TILE); + private static final String SETTING = QSTileHost.TILES_SETTING; @Mock private StatusBarIconController mIconController; @@ -107,8 +108,6 @@ public class QSTileHostTest extends SysuiTestCase { @Mock private DumpManager mDumpManager; @Mock - private BroadcastDispatcher mBroadcastDispatcher; - @Mock private QSTile.State mMockState; @Mock private CentralSurfaces mCentralSurfaces; @@ -132,31 +131,47 @@ public class QSTileHostTest extends SysuiTestCase { @Mock private TileLifecycleManager mTileLifecycleManager; - private Handler mHandler; - private TestableLooper mLooper; + private FakeExecutor mMainExecutor; + private QSTileHost mQSTileHost; @Before public void setUp() { MockitoAnnotations.initMocks(this); - mLooper = TestableLooper.get(this); - mHandler = new Handler(mLooper.getLooper()); + mMainExecutor = new FakeExecutor(new FakeSystemClock()); + when(mTileServiceRequestControllerBuilder.create(any())) .thenReturn(mTileServiceRequestController); when(mTileLifecycleManagerFactory.create(any(Intent.class), any(UserHandle.class))) .thenReturn(mTileLifecycleManager); mSecureSettings = new FakeSettings(); - mSecureSettings.putStringForUser( - QSTileHost.TILES_SETTING, "", "", false, mUserTracker.getUserId(), false); - mQSTileHost = new TestQSTileHost(mContext, mIconController, mDefaultFactory, mHandler, - mLooper.getLooper(), mPluginManager, mTunerService, mAutoTiles, mDumpManager, - mBroadcastDispatcher, mCentralSurfaces, mQSLogger, mUiEventLogger, mUserTracker, - mSecureSettings, mCustomTileStatePersister, mTileServiceRequestControllerBuilder, - mTileLifecycleManagerFactory); + saveSetting(""); + mQSTileHost = new TestQSTileHost(mContext, mIconController, mDefaultFactory, mMainExecutor, + mPluginManager, mTunerService, mAutoTiles, mDumpManager, mCentralSurfaces, + mQSLogger, mUiEventLogger, mUserTracker, mSecureSettings, mCustomTileStatePersister, + mTileServiceRequestControllerBuilder, mTileLifecycleManagerFactory); + + mSecureSettings.registerContentObserverForUser(SETTING, new ContentObserver(null) { + @Override + public void onChange(boolean selfChange) { + super.onChange(selfChange); + mMainExecutor.execute(() -> mQSTileHost.onTuningChanged(SETTING, getSetting())); + mMainExecutor.runAllReady(); + } + }, mUserTracker.getUserId()); setUpTileFactory(); } + private void saveSetting(String value) { + mSecureSettings.putStringForUser( + SETTING, value, "", false, mUserTracker.getUserId(), false); + } + + private String getSetting() { + return mSecureSettings.getStringForUser(SETTING, mUserTracker.getUserId()); + } + private void setUpTileFactory() { when(mMockState.toString()).thenReturn(MOCK_STATE_STRING); // Only create this kind of tiles @@ -173,6 +188,10 @@ public class QSTileHostTest extends SysuiTestCase { return new NotAvailableTile(mQSTileHost); } else if (CUSTOM_TILE_SPEC.equals(spec)) { return mCustomTile; + } else if ("internet".equals(spec) + || "wifi".equals(spec) + || "cell".equals(spec)) { + return new TestTile1(mQSTileHost); } else { return null; } @@ -196,14 +215,14 @@ public class QSTileHostTest extends SysuiTestCase { public void testInvalidSpecUsesDefault() { mContext.getOrCreateTestableResources() .addOverride(R.string.quick_settings_tiles, "spec1,spec2"); - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "not-valid"); + saveSetting("not-valid"); assertEquals(2, mQSTileHost.getTiles().size()); } @Test public void testRemoveWifiAndCellularWithoutInternet() { - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "wifi, spec1, cell, spec2"); + saveSetting("wifi, spec1, cell, spec2"); assertEquals("internet", mQSTileHost.mTileSpecs.get(0)); assertEquals("spec1", mQSTileHost.mTileSpecs.get(1)); @@ -212,7 +231,7 @@ public class QSTileHostTest extends SysuiTestCase { @Test public void testRemoveWifiAndCellularWithInternet() { - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "wifi, spec1, cell, spec2, internet"); + saveSetting("wifi, spec1, cell, spec2, internet"); assertEquals("spec1", mQSTileHost.mTileSpecs.get(0)); assertEquals("spec2", mQSTileHost.mTileSpecs.get(1)); @@ -221,7 +240,7 @@ public class QSTileHostTest extends SysuiTestCase { @Test public void testRemoveWifiWithoutInternet() { - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1, wifi, spec2"); + saveSetting("spec1, wifi, spec2"); assertEquals("spec1", mQSTileHost.mTileSpecs.get(0)); assertEquals("internet", mQSTileHost.mTileSpecs.get(1)); @@ -230,7 +249,7 @@ public class QSTileHostTest extends SysuiTestCase { @Test public void testRemoveCellWithInternet() { - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1, spec2, cell, internet"); + saveSetting("spec1, spec2, cell, internet"); assertEquals("spec1", mQSTileHost.mTileSpecs.get(0)); assertEquals("spec2", mQSTileHost.mTileSpecs.get(1)); @@ -239,7 +258,7 @@ public class QSTileHostTest extends SysuiTestCase { @Test public void testNoWifiNoCellularNoInternet() { - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1,spec2"); + saveSetting("spec1,spec2"); assertEquals("spec1", mQSTileHost.mTileSpecs.get(0)); assertEquals("spec2", mQSTileHost.mTileSpecs.get(1)); @@ -249,7 +268,7 @@ public class QSTileHostTest extends SysuiTestCase { public void testSpecWithInvalidDoesNotUseDefault() { mContext.getOrCreateTestableResources() .addOverride(R.string.quick_settings_tiles, "spec1,spec2"); - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec2,not-valid"); + saveSetting("spec2,not-valid"); assertEquals(1, mQSTileHost.getTiles().size()); QSTile element = CollectionUtils.firstOrNull(mQSTileHost.getTiles()); @@ -258,7 +277,7 @@ public class QSTileHostTest extends SysuiTestCase { @Test public void testDump() { - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1,spec2"); + saveSetting("spec1,spec2"); StringWriter w = new StringWriter(); PrintWriter pw = new PrintWriter(w); mQSTileHost.dump(pw, new String[]{}); @@ -274,7 +293,7 @@ public class QSTileHostTest extends SysuiTestCase { public void testDefault() { mContext.getOrCreateTestableResources() .addOverride(R.string.quick_settings_tiles_default, "spec1"); - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "default"); + saveSetting("default"); assertEquals(1, mQSTileHost.getTiles().size()); QSTile element = CollectionUtils.firstOrNull(mQSTileHost.getTiles()); assertTrue(element instanceof TestTile1); @@ -285,7 +304,7 @@ public class QSTileHostTest extends SysuiTestCase { public void testNoRepeatedSpecs_addTile() { mContext.getOrCreateTestableResources() .addOverride(R.string.quick_settings_tiles, "spec1,spec2"); - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1,spec2"); + saveSetting("spec1,spec2"); mQSTileHost.addTile("spec1"); @@ -298,9 +317,10 @@ public class QSTileHostTest extends SysuiTestCase { public void testAddTileAtValidPosition() { mContext.getOrCreateTestableResources() .addOverride(R.string.quick_settings_tiles, "spec1,spec3"); - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1,spec3"); + saveSetting("spec1,spec3"); mQSTileHost.addTile("spec2", 1); + mMainExecutor.runAllReady(); assertEquals(3, mQSTileHost.mTileSpecs.size()); assertEquals("spec1", mQSTileHost.mTileSpecs.get(0)); @@ -312,9 +332,10 @@ public class QSTileHostTest extends SysuiTestCase { public void testAddTileAtInvalidPositionAddsToEnd() { mContext.getOrCreateTestableResources() .addOverride(R.string.quick_settings_tiles, "spec1,spec3"); - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1,spec3"); + saveSetting("spec1,spec3"); mQSTileHost.addTile("spec2", 100); + mMainExecutor.runAllReady(); assertEquals(3, mQSTileHost.mTileSpecs.size()); assertEquals("spec1", mQSTileHost.mTileSpecs.get(0)); @@ -326,9 +347,10 @@ public class QSTileHostTest extends SysuiTestCase { public void testAddTileAtEnd() { mContext.getOrCreateTestableResources() .addOverride(R.string.quick_settings_tiles, "spec1,spec3"); - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1,spec3"); + saveSetting("spec1,spec3"); mQSTileHost.addTile("spec2", QSTileHost.POSITION_AT_END); + mMainExecutor.runAllReady(); assertEquals(3, mQSTileHost.mTileSpecs.size()); assertEquals("spec1", mQSTileHost.mTileSpecs.get(0)); @@ -338,9 +360,10 @@ public class QSTileHostTest extends SysuiTestCase { @Test public void testNoRepeatedSpecs_customTile() { - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, CUSTOM_TILE_SPEC); + saveSetting(CUSTOM_TILE_SPEC); mQSTileHost.addTile(CUSTOM_TILE, /* end */ false); + mMainExecutor.runAllReady(); assertEquals(1, mQSTileHost.mTileSpecs.size()); assertEquals(CUSTOM_TILE_SPEC, mQSTileHost.mTileSpecs.get(0)); @@ -348,9 +371,10 @@ public class QSTileHostTest extends SysuiTestCase { @Test public void testAddedAtBeginningOnDefault_customTile() { - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1"); // seed + saveSetting("spec1"); // seed mQSTileHost.addTile(CUSTOM_TILE); + mMainExecutor.runAllReady(); assertEquals(2, mQSTileHost.mTileSpecs.size()); assertEquals(CUSTOM_TILE_SPEC, mQSTileHost.mTileSpecs.get(0)); @@ -358,9 +382,10 @@ public class QSTileHostTest extends SysuiTestCase { @Test public void testAddedAtBeginning_customTile() { - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1"); // seed + saveSetting("spec1"); // seed mQSTileHost.addTile(CUSTOM_TILE, /* end */ false); + mMainExecutor.runAllReady(); assertEquals(2, mQSTileHost.mTileSpecs.size()); assertEquals(CUSTOM_TILE_SPEC, mQSTileHost.mTileSpecs.get(0)); @@ -368,9 +393,10 @@ public class QSTileHostTest extends SysuiTestCase { @Test public void testAddedAtEnd_customTile() { - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1"); // seed + saveSetting("spec1"); // seed mQSTileHost.addTile(CUSTOM_TILE, /* end */ true); + mMainExecutor.runAllReady(); assertEquals(2, mQSTileHost.mTileSpecs.size()); assertEquals(CUSTOM_TILE_SPEC, mQSTileHost.mTileSpecs.get(1)); @@ -409,13 +435,13 @@ public class QSTileHostTest extends SysuiTestCase { @Test public void testNotAvailableTile_specNotNull() { - mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "na"); + saveSetting("na"); verify(mQSLogger, never()).logTileDestroyed(isNull(), anyString()); } @Test public void testCustomTileRemoved_stateDeleted() { - mQSTileHost.changeTiles(List.of(CUSTOM_TILE_SPEC), List.of()); + mQSTileHost.changeTilesByUser(List.of(CUSTOM_TILE_SPEC), List.of()); verify(mCustomTileStatePersister) .removeState(new TileServiceKey(CUSTOM_TILE, mQSTileHost.getUserId())); @@ -423,29 +449,99 @@ public class QSTileHostTest extends SysuiTestCase { @Test public void testRemoveTiles() { - List tiles = List.of("spec1", "spec2", "spec3"); - mQSTileHost.saveTilesToSettings(tiles); + saveSetting("spec1,spec2,spec3"); mQSTileHost.removeTiles(List.of("spec1", "spec2")); + mMainExecutor.runAllReady(); assertEquals(List.of("spec3"), mQSTileHost.mTileSpecs); } + @Test + public void testTilesRemovedInQuickSuccession() { + saveSetting("spec1,spec2,spec3"); + mQSTileHost.removeTile("spec1"); + mQSTileHost.removeTile("spec3"); + + mMainExecutor.runAllReady(); + assertEquals(List.of("spec2"), mQSTileHost.mTileSpecs); + assertEquals("spec2", getSetting()); + } + + @Test + public void testAddTileInMainThread() { + saveSetting("spec1,spec2"); + + mQSTileHost.addTile("spec3"); + assertEquals(List.of("spec1", "spec2"), mQSTileHost.mTileSpecs); + + mMainExecutor.runAllReady(); + assertEquals(List.of("spec1", "spec2", "spec3"), mQSTileHost.mTileSpecs); + } + + @Test + public void testRemoveTileInMainThread() { + saveSetting("spec1,spec2"); + + mQSTileHost.removeTile("spec1"); + assertEquals(List.of("spec1", "spec2"), mQSTileHost.mTileSpecs); + + mMainExecutor.runAllReady(); + assertEquals(List.of("spec2"), mQSTileHost.mTileSpecs); + } + + @Test + public void testRemoveTilesInMainThread() { + saveSetting("spec1,spec2,spec3"); + + mQSTileHost.removeTiles(List.of("spec3", "spec1")); + assertEquals(List.of("spec1", "spec2", "spec3"), mQSTileHost.mTileSpecs); + + mMainExecutor.runAllReady(); + assertEquals(List.of("spec2"), mQSTileHost.mTileSpecs); + } + + @Test + public void testRemoveTileByUserInMainThread() { + saveSetting("spec1," + CUSTOM_TILE_SPEC); + + mQSTileHost.removeTileByUser(CUSTOM_TILE); + assertEquals(List.of("spec1", CUSTOM_TILE_SPEC), mQSTileHost.mTileSpecs); + + mMainExecutor.runAllReady(); + assertEquals(List.of("spec1"), mQSTileHost.mTileSpecs); + } + + @Test + public void testNonValidTileNotStoredInSettings() { + saveSetting("spec1,not-valid"); + + assertEquals(List.of("spec1"), mQSTileHost.mTileSpecs); + assertEquals("spec1", getSetting()); + } + + @Test + public void testNotAvailableTileNotStoredInSettings() { + saveSetting("spec1,na"); + + assertEquals(List.of("spec1"), mQSTileHost.mTileSpecs); + assertEquals("spec1", getSetting()); + } + private class TestQSTileHost extends QSTileHost { TestQSTileHost(Context context, StatusBarIconController iconController, - QSFactory defaultFactory, Handler mainHandler, Looper bgLooper, + QSFactory defaultFactory, Executor mainExecutor, PluginManager pluginManager, TunerService tunerService, Provider autoTiles, DumpManager dumpManager, - BroadcastDispatcher broadcastDispatcher, CentralSurfaces centralSurfaces, - QSLogger qsLogger, UiEventLogger uiEventLogger, UserTracker userTracker, - SecureSettings secureSettings, CustomTileStatePersister customTileStatePersister, + CentralSurfaces centralSurfaces, QSLogger qsLogger, UiEventLogger uiEventLogger, + UserTracker userTracker, SecureSettings secureSettings, + CustomTileStatePersister customTileStatePersister, TileServiceRequestController.Builder tileServiceRequestControllerBuilder, TileLifecycleManager.Factory tileLifecycleManagerFactory) { - super(context, iconController, defaultFactory, mainHandler, bgLooper, pluginManager, - tunerService, autoTiles, dumpManager, broadcastDispatcher, - Optional.of(centralSurfaces), qsLogger, uiEventLogger, userTracker, - secureSettings, customTileStatePersister, tileServiceRequestControllerBuilder, - tileLifecycleManagerFactory); + super(context, iconController, defaultFactory, mainExecutor, pluginManager, + tunerService, autoTiles, dumpManager, Optional.of(centralSurfaces), qsLogger, + uiEventLogger, userTracker, secureSettings, customTileStatePersister, + tileServiceRequestControllerBuilder, tileLifecycleManagerFactory); } @Override @@ -455,25 +551,16 @@ public class QSTileHostTest extends SysuiTestCase { @Override public void onPluginDisconnected(QSFactory plugin) { } - - @Override - void saveTilesToSettings(List tileSpecs) { - super.saveTilesToSettings(tileSpecs); - // After tiles are changed, make sure to call onTuningChanged with the new setting if it - // changed - String specs = mSecureSettings.getStringForUser( - QSTileHost.TILES_SETTING, mUserTracker.getUserId()); - onTuningChanged(TILES_SETTING, specs); - } } + private class TestTile extends QSTileImpl { protected TestTile(QSHost host) { super( host, - mLooper.getLooper(), - new Handler(mLooper.getLooper()), + mock(Looper.class), + mock(Handler.class), new FalsingManagerFake(), mock(MetricsLogger.class), mock(StatusBarStateController.class), diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileAdapterTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileAdapterTest.java index 3d53062d7d02f..d42cbe3b698a5 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileAdapterTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileAdapterTest.java @@ -55,6 +55,6 @@ public class TileAdapterTest extends SysuiTestCase { @Test public void testResetNotifiesHost() { mTileAdapter.resetTileSpecs(Collections.emptyList()); - verify(mQSTileHost).changeTiles(any(), any()); + verify(mQSTileHost).changeTilesByUser(any(), any()); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileServicesTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileServicesTest.java index 6b7e5b9335f21..471ddfd3f2243 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileServicesTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileServicesTest.java @@ -29,6 +29,7 @@ import static org.mockito.Mockito.when; import android.content.ComponentName; import android.content.Intent; import android.os.Handler; +import android.os.HandlerExecutor; import android.os.RemoteException; import android.os.UserHandle; import android.service.quicksettings.IQSTileService; @@ -65,6 +66,7 @@ import org.mockito.MockitoAnnotations; import java.util.ArrayList; import java.util.Optional; +import java.util.concurrent.Executor; import javax.inject.Provider; @@ -130,17 +132,16 @@ public class TileServicesTest extends SysuiTestCase { .thenReturn(mTileLifecycleManager); Provider provider = () -> new Handler(mTestableLooper.getLooper()); + Executor executor = new HandlerExecutor(provider.get()); QSTileHost host = new QSTileHost(mContext, mStatusBarIconController, mQSFactory, - provider.get(), - mTestableLooper.getLooper(), + executor, mPluginManager, mTunerService, () -> mAutoTileManager, mDumpManager, - mock(BroadcastDispatcher.class), Optional.of(mCentralSurfaces), mQSLogger, mUiEventLogger, 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 371119cc6d013..4ccbc6d45e638 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 @@ -490,7 +490,7 @@ public class AutoTileManagerTest extends SysuiTestCase { mAutoTileManager.init(); when(mAutoAddTracker.isAdded(TEST_CUSTOM_SAFETY_SPEC)).thenReturn(true); mAutoTileManager.mSafetyCallback.onSafetyCenterEnableChanged(false); - verify(mQsTileHost, times(1)).removeTile(safetyComponent); + verify(mQsTileHost, times(1)).removeTile(TEST_CUSTOM_SAFETY_SPEC); } @Test