From 6d9e26525b20b1991c2cff9244d73b507e9139b0 Mon Sep 17 00:00:00 2001 From: Valentin Iftime Date: Mon, 16 Oct 2023 09:29:17 +0200 Subject: Prioritize system toasts Insert toasts from system packages at the front of the queue to ensure that apps can't spam with toast to delay system toasts from showing. Also increase Clipboard paste warning toasts length to LENGTH_LONG. Test: atest NotificationManagerServiceTest Bug: 293301736 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:939612739c280b0204fe36d7549a77b94d55f3db) Merged-In: I13547f853476bc88d12026c545aba9f857ce8724 Change-Id: I13547f853476bc88d12026c545aba9f857ce8724 --- .../android/server/clipboard/ClipboardService.java | 4 +- .../notification/NotificationManagerService.java | 32 +++++++++- .../NotificationManagerServiceTest.java | 68 ++++++++++++++++++++++ 3 files changed, 100 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/clipboard/ClipboardService.java b/services/core/java/com/android/server/clipboard/ClipboardService.java index 4b8b43134e6c..b4e27d7f3f4e 100644 --- a/services/core/java/com/android/server/clipboard/ClipboardService.java +++ b/services/core/java/com/android/server/clipboard/ClipboardService.java @@ -1415,11 +1415,11 @@ public class ClipboardService extends SystemService { .getDrawable(R.drawable.ic_safety_protection); toastToShow = Toast.makeCustomToastWithIcon(toastContext, UiThread.get().getLooper(), message, - Toast.LENGTH_SHORT, safetyProtectionIcon); + Toast.LENGTH_LONG, safetyProtectionIcon); } else { toastToShow = Toast.makeText( toastContext, UiThread.get().getLooper(), message, - Toast.LENGTH_SHORT); + Toast.LENGTH_LONG); } toastToShow.show(); } diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index d997c8940b2f..20cf8c0e021d 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -3383,8 +3383,19 @@ public class NotificationManagerService extends SystemService { null /* options */); record = getToastRecord(callingUid, callingPid, pkg, isSystemToast, token, text, callback, duration, windowToken, displayId, textCallback); - mToastQueue.add(record); - index = mToastQueue.size() - 1; + + // Insert system toasts at the front of the queue + int systemToastInsertIdx = mToastQueue.size(); + if (isSystemToast) { + systemToastInsertIdx = getInsertIndexForSystemToastLocked(); + } + if (systemToastInsertIdx < mToastQueue.size()) { + index = systemToastInsertIdx; + mToastQueue.add(index, record); + } else { + mToastQueue.add(record); + index = mToastQueue.size() - 1; + } keepProcessAliveForToastIfNeededLocked(callingPid); } // If it's at index 0, it's the current toast. It doesn't matter if it's @@ -3400,6 +3411,23 @@ public class NotificationManagerService extends SystemService { } } + @GuardedBy("mToastQueue") + private int getInsertIndexForSystemToastLocked() { + // If there are other system toasts: insert after the last one + int idx = 0; + for (ToastRecord r : mToastQueue) { + if (idx == 0 && mIsCurrentToastShown) { + idx++; + continue; + } + if (!r.isSystemToast) { + return idx; + } + idx++; + } + return idx; + } + private boolean checkCanEnqueueToast(String pkg, int callingUid, int displayId, boolean isAppRenderedToast, boolean isSystemToast) { final boolean isPackageSuspended = isPackagePaused(pkg); 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 c31d0d778025..30501fc9b2ca 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -7594,6 +7594,74 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { assertEquals(NotificationManagerService.MAX_PACKAGE_TOASTS, mService.mToastQueue.size()); } + @Test + public void testPrioritizeSystemToasts() throws Exception { + // Insert non-system toasts + final String testPackage = "testPackageName"; + assertEquals(0, mService.mToastQueue.size()); + mService.isSystemUid = false; + mService.isSystemAppId = false; + setToastRateIsWithinQuota(true); + setIfPackageHasPermissionToAvoidToastRateLimiting(testPackage, false); + + // package is not suspended + when(mPackageManager.isPackageSuspendedForUser(testPackage, mUserId)) + .thenReturn(false); + + INotificationManager nmService = (INotificationManager) mService.mService; + + // Enqueue maximum number of toasts for test package + for (int i = 0; i < NotificationManagerService.MAX_PACKAGE_TOASTS; i++) { + enqueueTextToast(testPackage, "Text"); + } + + // Enqueue system toast + final String testPackageSystem = "testPackageNameSystem"; + mService.isSystemUid = true; + setIfPackageHasPermissionToAvoidToastRateLimiting(testPackageSystem, false); + when(mPackageManager.isPackageSuspendedForUser(testPackageSystem, mUserId)) + .thenReturn(false); + + enqueueToast(testPackageSystem, new TestableToastCallback()); + + // System toast is inserted at the front of the queue, behind current showing toast + assertEquals(testPackageSystem, mService.mToastQueue.get(1).pkg); + } + + @Test + public void testPrioritizeSystemToasts_enqueueAfterExistingSystemToast() throws Exception { + // Insert system toasts + final String testPackageSystem1 = "testPackageNameSystem1"; + assertEquals(0, mService.mToastQueue.size()); + mService.isSystemUid = true; + setToastRateIsWithinQuota(true); + setIfPackageHasPermissionToAvoidToastRateLimiting(testPackageSystem1, false); + + // package is not suspended + when(mPackageManager.isPackageSuspendedForUser(testPackageSystem1, mUserId)) + .thenReturn(false); + + INotificationManager nmService = (INotificationManager) mService.mService; + + // Enqueue maximum number of toasts for test package + for (int i = 0; i < NotificationManagerService.MAX_PACKAGE_TOASTS; i++) { + enqueueTextToast(testPackageSystem1, "Text"); + } + + // Enqueue another system toast + final String testPackageSystem2 = "testPackageNameSystem2"; + mService.isSystemUid = true; + setIfPackageHasPermissionToAvoidToastRateLimiting(testPackageSystem2, false); + when(mPackageManager.isPackageSuspendedForUser(testPackageSystem2, mUserId)) + .thenReturn(false); + + enqueueToast(testPackageSystem2, new TestableToastCallback()); + + // System toast is inserted at the back of the queue, after the other system toasts + assertEquals(testPackageSystem2, + mService.mToastQueue.get(mService.mToastQueue.size() - 1).pkg); + } + private void setAppInForegroundForToasts(int uid, boolean inForeground) { int importance = (inForeground) ? IMPORTANCE_FOREGROUND : IMPORTANCE_NONE; when(mActivityManager.getUidImportance(mUid)).thenReturn(importance); -- cgit v1.2.3 From 953d11ab8ea154da01e92bdf044a66fe108b42cd Mon Sep 17 00:00:00 2001 From: Valentin Iftime Date: Thu, 1 Feb 2024 13:58:49 +0100 Subject: Verify URI permission for channel sound update from NotificationListenerService Check that a privileged NotificationListenerService (CDM) has the permission to access the sound URI when updating a notification channel. Test: atest com.android.server.notification.NotificationManagerServiceTest#testUpdateNotificationChannelFromPrivilegedListener_noSoundUriPermission Bug: 317357401 (cherry picked from commit 9b7bbbf5ad542ecf9ecbf8cd819b468791b443c0) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:71cfb89a1cdaf743b7b67c724dfbbaa0cca98efc) Merged-In: Ic7d2e96e43565e98d2aa29b8f2ba35c142387ba9 Change-Id: Ic7d2e96e43565e98d2aa29b8f2ba35c142387ba9 --- .../notification/NotificationManagerService.java | 22 ++++++++ .../NotificationManagerServiceTest.java | 63 ++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 20cf8c0e021d..f1c2f7b92f5d 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -5772,6 +5772,10 @@ public class NotificationManagerService extends SystemService { Objects.requireNonNull(user); verifyPrivilegedListener(token, user, false); + + final NotificationChannel originalChannel = mPreferencesHelper.getNotificationChannel( + pkg, getUidForPackageAndUser(pkg, user), channel.getId(), true); + verifyPrivilegedListenerUriPermission(Binder.getCallingUid(), channel, originalChannel); updateNotificationChannelInt(pkg, getUidForPackageAndUser(pkg, user), channel, true); } @@ -5863,6 +5867,24 @@ public class NotificationManagerService extends SystemService { } } + private void verifyPrivilegedListenerUriPermission(int sourceUid, + @NonNull NotificationChannel updateChannel, + @Nullable NotificationChannel originalChannel) { + // Check that the NLS has the required permissions to access the channel + final Uri soundUri = updateChannel.getSound(); + final Uri originalSoundUri = + (originalChannel != null) ? originalChannel.getSound() : null; + if (soundUri != null && !Objects.equals(originalSoundUri, soundUri)) { + Binder.withCleanCallingIdentity(() -> { + mUgmInternal.checkGrantUriPermission(sourceUid, null, + ContentProvider.getUriWithoutUserId(soundUri), + Intent.FLAG_GRANT_READ_URI_PERMISSION, + ContentProvider.getUserIdFromUri(soundUri, + UserHandle.getUserId(sourceUid))); + }); + } + } + private int getUidForPackageAndUser(String pkg, UserHandle user) throws RemoteException { int uid = INVALID_UID; final long identity = Binder.clearCallingIdentity(); 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 30501fc9b2ca..d42700752ba9 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -3587,6 +3587,69 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { eq(NotificationListenerService.NOTIFICATION_CHANNEL_OR_GROUP_UPDATED)); } + @Test + public void testUpdateNotificationChannelFromPrivilegedListener_noSoundUriPermission() + throws Exception { + mService.setPreferencesHelper(mPreferencesHelper); + when(mCompanionMgr.getAssociations(PKG, mUserId)) + .thenReturn(singletonList(mock(AssociationInfo.class))); + when(mPreferencesHelper.getNotificationChannel(eq(PKG), anyInt(), + eq(mTestNotificationChannel.getId()), anyBoolean())) + .thenReturn(mTestNotificationChannel); + + final Uri soundUri = Uri.parse("content://media/test/sound/uri"); + final NotificationChannel updatedNotificationChannel = new NotificationChannel( + TEST_CHANNEL_ID, TEST_CHANNEL_ID, IMPORTANCE_DEFAULT); + updatedNotificationChannel.setSound(soundUri, + updatedNotificationChannel.getAudioAttributes()); + + doThrow(new SecurityException("no access")).when(mUgmInternal) + .checkGrantUriPermission(eq(Process.myUid()), any(), eq(soundUri), + anyInt(), eq(Process.myUserHandle().getIdentifier())); + + assertThrows(SecurityException.class, + () -> mBinderService.updateNotificationChannelFromPrivilegedListener(null, PKG, + Process.myUserHandle(), updatedNotificationChannel)); + + verify(mPreferencesHelper, never()).updateNotificationChannel( + anyString(), anyInt(), any(), anyBoolean(), anyInt(), anyBoolean()); + + verify(mListeners, never()).notifyNotificationChannelChanged(eq(PKG), + eq(Process.myUserHandle()), eq(mTestNotificationChannel), + eq(NotificationListenerService.NOTIFICATION_CHANNEL_OR_GROUP_UPDATED)); + } + + @Test + public void testUpdateNotificationChannelFromPrivilegedListener_noSoundUriPermission_sameSound() + throws Exception { + mService.setPreferencesHelper(mPreferencesHelper); + when(mCompanionMgr.getAssociations(PKG, mUserId)) + .thenReturn(singletonList(mock(AssociationInfo.class))); + when(mPreferencesHelper.getNotificationChannel(eq(PKG), anyInt(), + eq(mTestNotificationChannel.getId()), anyBoolean())) + .thenReturn(mTestNotificationChannel); + + final Uri soundUri = Settings.System.DEFAULT_NOTIFICATION_URI; + final NotificationChannel updatedNotificationChannel = new NotificationChannel( + TEST_CHANNEL_ID, TEST_CHANNEL_ID, IMPORTANCE_DEFAULT); + updatedNotificationChannel.setSound(soundUri, + updatedNotificationChannel.getAudioAttributes()); + + doThrow(new SecurityException("no access")).when(mUgmInternal) + .checkGrantUriPermission(eq(Process.myUid()), any(), eq(soundUri), + anyInt(), eq(Process.myUserHandle().getIdentifier())); + + mBinderService.updateNotificationChannelFromPrivilegedListener( + null, PKG, Process.myUserHandle(), updatedNotificationChannel); + + verify(mPreferencesHelper, times(1)).updateNotificationChannel( + anyString(), anyInt(), any(), anyBoolean(), anyInt(), anyBoolean()); + + verify(mListeners, never()).notifyNotificationChannelChanged(eq(PKG), + eq(Process.myUserHandle()), eq(mTestNotificationChannel), + eq(NotificationListenerService.NOTIFICATION_CHANNEL_OR_GROUP_UPDATED)); + } + @Test public void testGetNotificationChannelFromPrivilegedListener_cdm_success() throws Exception { mService.setPreferencesHelper(mPreferencesHelper); -- cgit v1.2.3 From 4995bb0e43d82cae6d8bf39c0b46e6c89df4bcd1 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Thu, 28 Mar 2024 20:48:11 +0530 Subject: change the initialization order for adding overlays ScreenDecorations adds "overlay" views to the window hierarchy, which house the appropriate decorations for various pieces of SystemUI. This change simply swaps the order in which we initalize those overlays such that the HWC overlay (special hardware-accelerated layers), if available, are always added last and are thus at the highest z-index. The reasoning for this is that some display pipelines are optimized to only process a HWC layer if it is on top. Note that this change means that any UI hosted in a software layer can potentially be covered by the HWC layer which is now on-top. This shouldn't be an issue in practice. Test: on device with HWC support, launch camera app and verify from SurfaceFlinger dumpsys that the HWC layers have the highest z-index Bug: 313038036 Flag: NONE Change-Id: I2e88774db0a40918c7f0be580ff66fe7a2093798 CRs-Fixed: 3657031 --- .../src/com/android/systemui/ScreenDecorations.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/ScreenDecorations.java b/packages/SystemUI/src/com/android/systemui/ScreenDecorations.java index 4dc227c91177..d3196f6c03e1 100644 --- a/packages/SystemUI/src/com/android/systemui/ScreenDecorations.java +++ b/packages/SystemUI/src/com/android/systemui/ScreenDecorations.java @@ -566,12 +566,8 @@ public class ScreenDecorations implements CoreStartable, Tunable , Dumpable { List decorProviders = getProviders(mHwcScreenDecorationSupport != null); removeRedundantOverlayViews(decorProviders); - if (mHwcScreenDecorationSupport != null) { - createHwcOverlay(); - } else { - removeHwcOverlay(); - } - + // Overlays are added in 2 steps: first the standard overlays. Then, if applicable, the + // HWC overlays. This ensures that the HWC overlays are always on top boolean[] hasCreatedOverlay = new boolean[BOUNDS_POSITION_LENGTH]; final boolean shouldOptimizeVisibility = shouldOptimizeVisibility(); Integer bound; @@ -588,6 +584,13 @@ public class ScreenDecorations implements CoreStartable, Tunable , Dumpable { } } + // Adding the HWC overlays second so they are on top by default + if (mHwcScreenDecorationSupport != null) { + createHwcOverlay(); + } else { + removeHwcOverlay(); + } + if (shouldOptimizeVisibility) { mDotViewController.setShowingListener(mPrivacyDotShowingListener); } else { -- cgit v1.2.3