Merge "Exempt the default browser from trampoline restrictions" into sc-dev

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),
+            RoleObserver roleObserver = new RoleObserver(getContext(),
-                    mPackageManager, getContext().getMainExecutor());
+                    getContext().getSystemService(RoleManager.class),
-            mRoleObserver.init();
+                    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,
+        RoleObserver(Context context, @NonNull RoleManager roleManager,
-                @NonNull IPackageManager pkgMgr,
+                @NonNull IPackageManager pkgMgr, @NonNull Looper mainLooper) {
-                @NonNull @CallbackExecutor Executor executor) {
             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

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);
 

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 UserHandle(0));
-        mUsers.add(new UserInfo(10, "second", 0));
+        mUsers.add(new UserHandle(10));
-        when(mUm.getUsers()).thenReturn(mUsers);
+        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())).
+                mUsers.get(0)))
-                thenReturn(dialer0);
+                .thenReturn(dialer0);
         when(mRoleManager.getRoleHoldersAsUser(
                 ROLE_EMERGENCY,
-                mUsers.get(0).getUserHandle())).
+                mUsers.get(0)))
-                thenReturn(emer0);
+                .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())).
+                mUsers.get(0)))
-                thenReturn(dialer0);
+                .thenReturn(dialer0);
 
         mRoleObserver.init();
 
@@ -241,8 +262,8 @@ public class RoleObserverTest extends UiServiceTestCase {
 
         when(mRoleManager.getRoleHoldersAsUser(
                 ROLE_DIALER,
-                mUsers.get(0).getUserHandle())).
+                mUsers.get(0)))
-                thenReturn(newDefault);
+                .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())).
+                mUsers.get(0)))
-                thenReturn(dialer0);
+                .thenReturn(dialer0);
 
         mRoleObserver.init();
 
@@ -273,8 +318,8 @@ public class RoleObserverTest extends UiServiceTestCase {
 
         when(mRoleManager.getRoleHoldersAsUser(
                 ROLE_DIALER,
-                mUsers.get(0).getUserHandle())).
+                mUsers.get(0)))
-                thenReturn(newDefault);
+                .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())).
+                mUsers.get(0)))
-                thenReturn(dialer0);
+                .thenReturn(dialer0);
 
         mRoleObserver.init();
 
@@ -310,8 +355,8 @@ public class RoleObserverTest extends UiServiceTestCase {
 
         when(mRoleManager.getRoleHoldersAsUser(
                 ROLE_DIALER,
-                mUsers.get(1).getUserHandle())).
+                mUsers.get(1)))
-                thenReturn(dialer10);
+                .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));
+    }
 }