diff options
3 files changed, 195 insertions, 48 deletions
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 9fd79cefa19c..0840e75823b5 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -123,7 +123,7 @@ import static com.android.server.utils.PriorityDump.PRIORITY_ARG_NORMAL; import android.Manifest; import android.Manifest.permission; -import android.annotation.CallbackExecutor; +import android.annotation.MainThread; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.RequiresPermission; @@ -492,7 +492,7 @@ public class NotificationManagerService extends SystemService { private DeviceIdleManager mDeviceIdleManager; private IUriGrantsManager mUgm; private UriGrantsManagerInternal mUgmInternal; - private RoleObserver mRoleObserver; + private volatile RoleObserver mRoleObserver; private UserManager mUm; private IPlatformCompat mPlatformCompat; private ShortcutHelper mShortcutHelper; @@ -2651,6 +2651,11 @@ public class NotificationManagerService extends SystemService { @Override public void onBootPhase(int phase) { + onBootPhase(phase, Looper.getMainLooper()); + } + + @VisibleForTesting + void onBootPhase(int phase, Looper mainLooper) { if (phase == SystemService.PHASE_SYSTEM_SERVICES_READY) { // no beeping until we're basically done booting mSystemReady = true; @@ -2660,9 +2665,11 @@ public class NotificationManagerService extends SystemService { mAudioManagerInternal = getLocalService(AudioManagerInternal.class); mWindowManagerInternal = LocalServices.getService(WindowManagerInternal.class); mZenModeHelper.onSystemReady(); - mRoleObserver = new RoleObserver(getContext().getSystemService(RoleManager.class), - mPackageManager, getContext().getMainExecutor()); - mRoleObserver.init(); + RoleObserver roleObserver = new RoleObserver(getContext(), + getContext().getSystemService(RoleManager.class), + mPackageManager, mainLooper); + roleObserver.init(); + mRoleObserver = roleObserver; LauncherApps launcherApps = (LauncherApps) getContext().getSystemService(Context.LAUNCHER_APPS_SERVICE); mShortcutHelper = new ShortcutHelper(launcherApps, mShortcutListener, getLocalService( @@ -10688,26 +10695,40 @@ public class NotificationManagerService extends SystemService { // Role name : user id : list of approved packages private ArrayMap<String, ArrayMap<Integer, ArraySet<String>>> mNonBlockableDefaultApps; + /** + * Writes should be pretty rare (only when default browser changes) and reads are done + * during activity start code-path, so we're optimizing for reads. This means this set is + * immutable once written and we'll recreate the set every time there is a role change and + * then assign that new set to the volatile below, so reads can be done without needing to + * hold a lock. Every write is done on the main-thread, so write atomicity is guaranteed. + * + * Didn't use unmodifiable set to enforce immutability to avoid iterating via iterators. + */ + private volatile ArraySet<Integer> mTrampolineExemptUids = new ArraySet<>(); + private final RoleManager mRm; private final IPackageManager mPm; private final Executor mExecutor; + private final Looper mMainLooper; - RoleObserver(@NonNull RoleManager roleManager, - @NonNull IPackageManager pkgMgr, - @NonNull @CallbackExecutor Executor executor) { + RoleObserver(Context context, @NonNull RoleManager roleManager, + @NonNull IPackageManager pkgMgr, @NonNull Looper mainLooper) { mRm = roleManager; mPm = pkgMgr; - mExecutor = executor; + mExecutor = context.getMainExecutor(); + mMainLooper = mainLooper; } + /** Should be called from the main-thread. */ + @MainThread public void init() { - List<UserInfo> users = mUm.getUsers(); + List<UserHandle> users = mUm.getUserHandles(/* excludeDying */ true); mNonBlockableDefaultApps = new ArrayMap<>(); for (int i = 0; i < NON_BLOCKABLE_DEFAULT_ROLES.length; i++) { final ArrayMap<Integer, ArraySet<String>> userToApprovedList = new ArrayMap<>(); mNonBlockableDefaultApps.put(NON_BLOCKABLE_DEFAULT_ROLES[i], userToApprovedList); for (int j = 0; j < users.size(); j++) { - Integer userId = users.get(j).getUserHandle().getIdentifier(); + Integer userId = users.get(j).getIdentifier(); ArraySet<String> approvedForUserId = new ArraySet<>(mRm.getRoleHoldersAsUser( NON_BLOCKABLE_DEFAULT_ROLES[i], UserHandle.of(userId))); ArraySet<Pair<String, Integer>> approvedAppUids = new ArraySet<>(); @@ -10718,7 +10739,7 @@ public class NotificationManagerService extends SystemService { mPreferencesHelper.updateDefaultApps(userId, null, approvedAppUids); } } - + updateTrampolineExemptUidsForUsers(users.toArray(new UserHandle[0])); mRm.addOnRoleHoldersChangedListenerAsUser(mExecutor, this, UserHandle.ALL); } @@ -10727,6 +10748,11 @@ public class NotificationManagerService extends SystemService { return mNonBlockableDefaultApps.get(role).get(userId).contains(pkg); } + @VisibleForTesting + public boolean isUidExemptFromTrampolineRestrictions(int uid) { + return mTrampolineExemptUids.contains(uid); + } + /** * Convert the assistant-role holder into settings. The rest of the system uses the * settings. @@ -10736,6 +10762,12 @@ public class NotificationManagerService extends SystemService { */ @Override public void onRoleHoldersChanged(@NonNull String roleName, @NonNull UserHandle user) { + onRoleHoldersChangedForNonBlockableDefaultApps(roleName, user); + onRoleHoldersChangedForTrampolines(roleName, user); + } + + private void onRoleHoldersChangedForNonBlockableDefaultApps(@NonNull String roleName, + @NonNull UserHandle user) { // we only care about a couple of the roles they'll tell us about boolean relevantChange = false; for (int i = 0; i < NON_BLOCKABLE_DEFAULT_ROLES.length; i++) { @@ -10783,6 +10815,41 @@ public class NotificationManagerService extends SystemService { // write of the notification policy xml for this change } + private void onRoleHoldersChangedForTrampolines(@NonNull String roleName, + @NonNull UserHandle user) { + if (!RoleManager.ROLE_BROWSER.equals(roleName)) { + return; + } + updateTrampolineExemptUidsForUsers(user); + } + + private void updateTrampolineExemptUidsForUsers(UserHandle... users) { + Preconditions.checkState(mMainLooper.isCurrentThread()); + ArraySet<Integer> oldUids = mTrampolineExemptUids; + ArraySet<Integer> newUids = new ArraySet<>(); + // Add the uids from previous set for the users that we won't update. + for (int i = 0, n = oldUids.size(); i < n; i++) { + int uid = oldUids.valueAt(i); + UserHandle user = UserHandle.of(UserHandle.getUserId(uid)); + if (!ArrayUtils.contains(users, user)) { + newUids.add(uid); + } + } + // Now lookup the new uids for the users that we want to update. + for (int i = 0, n = users.length; i < n; i++) { + UserHandle user = users[i]; + for (String pkg : mRm.getRoleHoldersAsUser(RoleManager.ROLE_BROWSER, user)) { + int uid = getUidForPackage(pkg, user.getIdentifier()); + if (uid != -1) { + newUids.add(uid); + } else { + Slog.e(TAG, "Bad uid (-1) for browser package " + pkg); + } + } + } + mTrampolineExemptUids = newUids; + } + private int getUidForPackage(String pkg, int userId) { try { return mPm.getPackageUid(pkg, MATCH_ALL, userId); @@ -10947,7 +11014,7 @@ public class NotificationManagerService extends SystemService { } String logcatMessage = "Indirect notification activity start (trampoline) from " + packageName; - if (CompatChanges.isChangeEnabled(NOTIFICATION_TRAMPOLINE_BLOCK, uid)) { + if (blockTrampoline(uid)) { // Post toast() call to mHandler to offload PM lookup from the activity start path mHandler.post(() -> toast(packageName, uid)); Slog.e(TAG, logcatMessage + " blocked"); @@ -10958,6 +11025,13 @@ public class NotificationManagerService extends SystemService { } } + private boolean blockTrampoline(int uid) { + if (mRoleObserver != null && mRoleObserver.isUidExemptFromTrampolineRestrictions(uid)) { + return false; + } + return CompatChanges.isChangeEnabled(NOTIFICATION_TRAMPOLINE_BLOCK, uid); + } + @Override public boolean canCloseSystemDialogs(Collection<IBinder> tokens, int uid) { // If the start is allowed via notification, we allow the app to close system dialogs 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 537bc2c7d52c..c33287c57377 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -141,6 +141,7 @@ import android.os.Binder; import android.os.Build; import android.os.Bundle; import android.os.IBinder; +import android.os.Looper; import android.os.Parcel; import android.os.Process; import android.os.RemoteException; @@ -267,6 +268,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { RankingHandler mRankingHandler; @Mock ActivityManagerInternal mAmi; + @Mock + private Looper mMainLooper; @Mock IIntentSender pi1; @@ -514,7 +517,9 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mAppUsageStats, mock(DevicePolicyManagerInternal.class), mUgm, mUgmInternal, mAppOpsManager, mUm, mHistoryManager, mStatsManager, mock(TelephonyManager.class), mAmi, mToastRateLimiter); - mService.onBootPhase(SystemService.PHASE_SYSTEM_SERVICES_READY); + // Return first true for RoleObserver main-thread check + when(mMainLooper.isCurrentThread()).thenReturn(true).thenReturn(false); + mService.onBootPhase(SystemService.PHASE_SYSTEM_SERVICES_READY, mMainLooper); mService.setAudioManager(mAudioManager); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java b/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java index 4ce237e3aadc..27ae46c87b28 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/RoleObserverTest.java @@ -16,6 +16,7 @@ package com.android.server.notification; +import static android.app.role.RoleManager.ROLE_BROWSER; import static android.app.role.RoleManager.ROLE_DIALER; import static android.app.role.RoleManager.ROLE_EMERGENCY; import static android.content.pm.PackageManager.MATCH_ALL; @@ -23,6 +24,7 @@ import static android.content.pm.PackageManager.MATCH_ALL; import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; @@ -31,6 +33,8 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static java.util.Arrays.asList; + import android.app.ActivityManager; import android.app.ActivityManagerInternal; import android.app.AppOpsManager; @@ -44,7 +48,6 @@ import android.companion.ICompanionDeviceManager; import android.content.Context; import android.content.pm.IPackageManager; import android.content.pm.PackageManager; -import android.content.pm.UserInfo; import android.os.Looper; import android.os.UserHandle; import android.os.UserManager; @@ -80,7 +83,6 @@ import org.mockito.MockitoAnnotations; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.Executor; @SmallTest @RunWith(AndroidTestingRunner.class) @@ -98,13 +100,13 @@ public class RoleObserverTest extends UiServiceTestCase { @Mock private UserManager mUm; @Mock - private Executor mExecutor; - @Mock private RoleManager mRoleManager; + @Mock + private Looper mMainLooper; NotificationRecordLoggerFake mNotificationRecordLogger = new NotificationRecordLoggerFake(); private InstanceIdSequence mNotificationInstanceIdSequence = new InstanceIdSequenceFake( 1 << 30); - private List<UserInfo> mUsers; + private List<UserHandle> mUsers; private static class TestableNotificationManagerService extends NotificationManagerService { TestableNotificationManagerService(Context context, @@ -133,13 +135,15 @@ public class RoleObserverTest extends UiServiceTestCase { mContext.addMockSystemService(AppOpsManager.class, mock(AppOpsManager.class)); mUsers = new ArrayList<>(); - mUsers.add(new UserInfo(0, "system", 0)); - mUsers.add(new UserInfo(10, "second", 0)); - when(mUm.getUsers()).thenReturn(mUsers); + mUsers.add(new UserHandle(0)); + mUsers.add(new UserHandle(10)); + when(mUm.getUserHandles(anyBoolean())).thenReturn(mUsers); + + when(mMainLooper.isCurrentThread()).thenReturn(true); mService = new TestableNotificationManagerService(mContext, mNotificationRecordLogger, mNotificationInstanceIdSequence); - mRoleObserver = mService.new RoleObserver(mRoleManager, mPm, mExecutor); + mRoleObserver = mService.new RoleObserver(mContext, mRoleManager, mPm, mMainLooper); try { mService.init(mService.new WorkerHandler(mock(Looper.class)), @@ -174,7 +178,7 @@ public class RoleObserverTest extends UiServiceTestCase { } @Test - public void testInit() throws Exception { + public void testInit_forNonBlockableDefaultApps() throws Exception { List<String> dialer0 = new ArrayList<>(); dialer0.add("dialer"); List<String> emer0 = new ArrayList<>(); @@ -191,29 +195,29 @@ public class RoleObserverTest extends UiServiceTestCase { when(mRoleManager.getRoleHoldersAsUser( ROLE_DIALER, - mUsers.get(0).getUserHandle())). - thenReturn(dialer0); + mUsers.get(0))) + .thenReturn(dialer0); when(mRoleManager.getRoleHoldersAsUser( ROLE_EMERGENCY, - mUsers.get(0).getUserHandle())). - thenReturn(emer0); + mUsers.get(0))) + .thenReturn(emer0); mRoleObserver.init(); // verify internal records of current state of the world assertTrue(mRoleObserver.isApprovedPackageForRoleForUser( - ROLE_DIALER, dialer0.get(0), mUsers.get(0).id)); + ROLE_DIALER, dialer0.get(0), mUsers.get(0).getIdentifier())); assertFalse(mRoleObserver.isApprovedPackageForRoleForUser( - ROLE_DIALER, dialer0.get(0), mUsers.get(1).id)); + ROLE_DIALER, dialer0.get(0), mUsers.get(1).getIdentifier())); assertTrue(mRoleObserver.isApprovedPackageForRoleForUser( - ROLE_EMERGENCY, emer0.get(0), mUsers.get(0).id)); + ROLE_EMERGENCY, emer0.get(0), mUsers.get(0).getIdentifier())); assertFalse(mRoleObserver.isApprovedPackageForRoleForUser( - ROLE_EMERGENCY, emer0.get(0), mUsers.get(1).id)); + ROLE_EMERGENCY, emer0.get(0), mUsers.get(1).getIdentifier())); // make sure we're listening to updates verify(mRoleManager, times(1)).addOnRoleHoldersChangedListenerAsUser( - eq(mExecutor), any(), eq(UserHandle.ALL)); + any(), any(), eq(UserHandle.ALL)); // make sure we told pref helper about the state of the world verify(mPreferencesHelper, times(1)).updateDefaultApps(0, null, dialer0Pair); @@ -221,14 +225,31 @@ public class RoleObserverTest extends UiServiceTestCase { } @Test - public void testSwapDefault() throws Exception { + public void testInit_forTrampolines() throws Exception { + when(mPm.getPackageUid("com.browser", MATCH_ALL, 0)).thenReturn(30); + when(mRoleManager.getRoleHoldersAsUser( + ROLE_BROWSER, + mUsers.get(0))) + .thenReturn(asList("com.browser")); + + mRoleObserver.init(); + + assertTrue(mRoleObserver.isUidExemptFromTrampolineRestrictions(30)); + + // make sure we're listening to updates + verify(mRoleManager, times(1)).addOnRoleHoldersChangedListenerAsUser(any(), any(), + eq(UserHandle.ALL)); + } + + @Test + public void testSwapDefault_forNonBlockableDefaultApps() throws Exception { List<String> dialer0 = new ArrayList<>(); dialer0.add("dialer"); when(mRoleManager.getRoleHoldersAsUser( ROLE_DIALER, - mUsers.get(0).getUserHandle())). - thenReturn(dialer0); + mUsers.get(0))) + .thenReturn(dialer0); mRoleObserver.init(); @@ -241,8 +262,8 @@ public class RoleObserverTest extends UiServiceTestCase { when(mRoleManager.getRoleHoldersAsUser( ROLE_DIALER, - mUsers.get(0).getUserHandle())). - thenReturn(newDefault); + mUsers.get(0))) + .thenReturn(newDefault); mRoleObserver.onRoleHoldersChanged(ROLE_DIALER, UserHandle.of(0)); @@ -251,15 +272,39 @@ public class RoleObserverTest extends UiServiceTestCase { } @Test - public void testSwapDefault_multipleOverlappingApps() throws Exception { + public void testSwapDefault_forTrampolines() throws Exception { + List<String> dialer0 = new ArrayList<>(); + when(mPm.getPackageUid("com.browser", MATCH_ALL, 0)).thenReturn(30); + when(mPm.getPackageUid("com.browser2", MATCH_ALL, 0)).thenReturn(31); + when(mRoleManager.getRoleHoldersAsUser( + ROLE_BROWSER, + mUsers.get(0))) + .thenReturn(asList("com.browser")); + mRoleObserver.init(); + assertTrue(mRoleObserver.isUidExemptFromTrampolineRestrictions(30)); + assertFalse(mRoleObserver.isUidExemptFromTrampolineRestrictions(31)); + // Default changed + when(mRoleManager.getRoleHoldersAsUser( + ROLE_BROWSER, + mUsers.get(0))) + .thenReturn(asList("com.browser2")); + mRoleObserver.onRoleHoldersChanged(ROLE_BROWSER, UserHandle.of(0)); + + assertFalse(mRoleObserver.isUidExemptFromTrampolineRestrictions(30)); + assertTrue(mRoleObserver.isUidExemptFromTrampolineRestrictions(31)); + } + + @Test + public void testSwapDefault_multipleOverlappingApps_forNonBlockableDefaultApps() + throws Exception { List<String> dialer0 = new ArrayList<>(); dialer0.add("dialer"); dialer0.add("phone"); when(mRoleManager.getRoleHoldersAsUser( ROLE_DIALER, - mUsers.get(0).getUserHandle())). - thenReturn(dialer0); + mUsers.get(0))) + .thenReturn(dialer0); mRoleObserver.init(); @@ -273,8 +318,8 @@ public class RoleObserverTest extends UiServiceTestCase { when(mRoleManager.getRoleHoldersAsUser( ROLE_DIALER, - mUsers.get(0).getUserHandle())). - thenReturn(newDefault); + mUsers.get(0))) + .thenReturn(newDefault); ArraySet<String> expectedRemove = new ArraySet<>(); expectedRemove.add("dialer"); @@ -294,14 +339,14 @@ public class RoleObserverTest extends UiServiceTestCase { } @Test - public void testSwapDefault_newUser() throws Exception { + public void testSwapDefault_newUser_forNonBlockableDefaultApps() throws Exception { List<String> dialer0 = new ArrayList<>(); dialer0.add("dialer"); when(mRoleManager.getRoleHoldersAsUser( ROLE_DIALER, - mUsers.get(0).getUserHandle())). - thenReturn(dialer0); + mUsers.get(0))) + .thenReturn(dialer0); mRoleObserver.init(); @@ -310,8 +355,8 @@ public class RoleObserverTest extends UiServiceTestCase { when(mRoleManager.getRoleHoldersAsUser( ROLE_DIALER, - mUsers.get(1).getUserHandle())). - thenReturn(dialer10); + mUsers.get(1))) + .thenReturn(dialer10); ArraySet<Pair<String, Integer>> expectedAddPair = new ArraySet<>(); expectedAddPair.add(new Pair("phone", 30)); @@ -329,4 +374,27 @@ public class RoleObserverTest extends UiServiceTestCase { assertTrue(mRoleObserver.isApprovedPackageForRoleForUser(ROLE_DIALER, "phone", 10)); assertTrue(mRoleObserver.isApprovedPackageForRoleForUser(ROLE_DIALER, "dialer", 0)); } + + @Test + public void testSwapDefault_newUser_forTrampolines() throws Exception { + List<String> dialer0 = new ArrayList<>(); + when(mPm.getPackageUid("com.browser", MATCH_ALL, 0)).thenReturn(30); + when(mPm.getPackageUid("com.browser2", MATCH_ALL, 10)).thenReturn(1031); + when(mRoleManager.getRoleHoldersAsUser( + ROLE_BROWSER, + mUsers.get(0))) + .thenReturn(asList("com.browser")); + mRoleObserver.init(); + assertTrue(mRoleObserver.isUidExemptFromTrampolineRestrictions(30)); + assertFalse(mRoleObserver.isUidExemptFromTrampolineRestrictions(1031)); + // New user + when(mRoleManager.getRoleHoldersAsUser( + ROLE_BROWSER, + mUsers.get(1))) + .thenReturn(asList("com.browser2")); + mRoleObserver.onRoleHoldersChanged(ROLE_BROWSER, UserHandle.of(10)); + + assertTrue(mRoleObserver.isUidExemptFromTrampolineRestrictions(30)); + assertTrue(mRoleObserver.isUidExemptFromTrampolineRestrictions(1031)); + } } |