Merge "Revert "Clear notifications for packages whose permission is revoked"" into udc-qpr-dev am: e0bd1d5daf am: 99f0f319d3

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/24130123

Change-Id: I0716228403ce49d398e6f4a6e800917edc1b75f5
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

2 files changed, 49 insertions, 163 deletions

diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index 2db724f51918..8e1ad6529bc0 100644
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -2491,16 +2491,6 @@ public class NotificationManagerService extends SystemService {
         getContext().registerReceiver(mReviewNotificationPermissionsReceiver,
                 ReviewNotificationPermissionsReceiver.getFilter(),
                 Context.RECEIVER_NOT_EXPORTED);
-
-        mAppOps.startWatchingMode(AppOpsManager.OP_POST_NOTIFICATION, null,
-                new AppOpsManager.OnOpChangedInternalListener() {
-                    @Override
-                    public void onOpChanged(@NonNull String op, @NonNull String packageName,
-                            int userId) {
-                        mHandler.post(
-                                () -> handleNotificationPermissionChange(packageName, userId));
-                    }
-                });
     }
 
     /**
@@ -3560,16 +3550,20 @@ public class NotificationManagerService extends SystemService {
             }
             mPermissionHelper.setNotificationPermission(
                     pkg, UserHandle.getUserId(uid), enabled, true);
+            sendAppBlockStateChangedBroadcast(pkg, uid, !enabled);
 
             mMetricsLogger.write(new LogMaker(MetricsEvent.ACTION_BAN_APP_NOTES)
                     .setType(MetricsEvent.TYPE_ACTION)
                     .setPackageName(pkg)
                     .setSubtype(enabled ? 1 : 0));
             mNotificationChannelLogger.logAppNotificationsAllowed(uid, pkg, enabled);
+            // Now, cancel any outstanding notifications that are part of a just-disabled app
+            if (!enabled) {
+                cancelAllNotificationsInt(MY_UID, MY_PID, pkg, null, 0, 0,
+                        UserHandle.getUserId(uid), REASON_PACKAGE_BANNED);
+            }
 
-            // Outstanding notifications from this package will be cancelled, and the package will
-            // be sent the ACTION_APP_BLOCK_STATE_CHANGED broadcast, as soon as we get the
-            // callback from AppOpsManager.
+            handleSavePolicyFile();
         }
 
         /**
@@ -5889,21 +5883,6 @@ public class NotificationManagerService extends SystemService {
         }
     };
 
-    private void handleNotificationPermissionChange(String pkg, @UserIdInt int userId) {
-        int uid = mPackageManagerInternal.getPackageUid(pkg, 0, userId);
-        if (uid == INVALID_UID) {
-            Log.e(TAG, String.format("No uid found for %s, %s!", pkg, userId));
-            return;
-        }
-        boolean hasPermission = mPermissionHelper.hasPermission(uid);
-        sendAppBlockStateChangedBroadcast(pkg, uid, !hasPermission);
-        if (!hasPermission) {
-            cancelAllNotificationsInt(MY_UID, MY_PID, pkg, /* channelId= */ null,
-                    /* mustHaveFlags= */ 0, /* mustNotHaveFlags= */ 0, userId,
-                    REASON_PACKAGE_BANNED);
-        }
-    }
-
     protected void checkNotificationListenerAccess() {
         if (!isCallerSystemOrPhone()) {
             getContext().enforceCallingPermission(
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 a109d5cddd21..f552ab2dab60 100755
--- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
@@ -406,7 +406,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
     UriGrantsManagerInternal mUgmInternal;
     @Mock
     AppOpsManager mAppOpsManager;
-    private AppOpsManager.OnOpChangedListener mOnPermissionChangeListener;
     @Mock
     private TestableNotificationManagerService.NotificationAssistantAccessGrantedCallback
             mNotificationAssistantAccessGrantedCallback;
@@ -606,12 +605,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
         tr.addOverride(com.android.internal.R.string.config_defaultSearchSelectorPackageName,
                 SEARCH_SELECTOR_PKG);
 
-        doAnswer(invocation -> {
-            mOnPermissionChangeListener = invocation.getArgument(2);
-            return null;
-        }).when(mAppOpsManager).startWatchingMode(eq(AppOpsManager.OP_POST_NOTIFICATION), any(),
-                any());
-
         mWorkerHandler = spy(mService.new WorkerHandler(mTestableLooper.getLooper()));
         mService.init(mWorkerHandler, mRankingHandler, mPackageManager, mPackageManagerClient,
                 mockLightsManager, mListeners, mAssistants, mConditionProviders, mCompanionMgr,
@@ -3224,6 +3217,48 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
     }
 
     @Test
+    public void testUpdateAppNotifyCreatorBlock() throws Exception {
+        when(mPermissionHelper.hasPermission(mUid)).thenReturn(true);
+
+        mBinderService.setNotificationsEnabledForPackage(PKG, mUid, false);
+        Thread.sleep(500);
+        waitForIdle();
+
+        ArgumentCaptor<Intent> captor = ArgumentCaptor.forClass(Intent.class);
+        verify(mContext, times(1)).sendBroadcastAsUser(captor.capture(), any(), eq(null));
+
+        assertEquals(NotificationManager.ACTION_APP_BLOCK_STATE_CHANGED,
+                captor.getValue().getAction());
+        assertEquals(PKG, captor.getValue().getPackage());
+        assertTrue(captor.getValue().getBooleanExtra(EXTRA_BLOCKED_STATE, true));
+    }
+
+    @Test
+    public void testUpdateAppNotifyCreatorBlock_notIfMatchesExistingSetting() throws Exception {
+        when(mPermissionHelper.hasPermission(mUid)).thenReturn(false);
+
+        mBinderService.setNotificationsEnabledForPackage(PKG, 0, false);
+        verify(mContext, never()).sendBroadcastAsUser(any(), any(), eq(null));
+    }
+
+    @Test
+    public void testUpdateAppNotifyCreatorUnblock() throws Exception {
+        when(mPermissionHelper.hasPermission(mUid)).thenReturn(false);
+
+        mBinderService.setNotificationsEnabledForPackage(PKG, mUid, true);
+        Thread.sleep(500);
+        waitForIdle();
+
+        ArgumentCaptor<Intent> captor = ArgumentCaptor.forClass(Intent.class);
+        verify(mContext, times(1)).sendBroadcastAsUser(captor.capture(), any(), eq(null));
+
+        assertEquals(NotificationManager.ACTION_APP_BLOCK_STATE_CHANGED,
+                captor.getValue().getAction());
+        assertEquals(PKG, captor.getValue().getPackage());
+        assertFalse(captor.getValue().getBooleanExtra(EXTRA_BLOCKED_STATE, true));
+    }
+
+    @Test
     public void testUpdateChannelNotifyCreatorBlock() throws Exception {
         mService.setPreferencesHelper(mPreferencesHelper);
         when(mPreferencesHelper.getNotificationChannel(eq(PKG), anyInt(),
@@ -12139,134 +12174,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
                 any(), eq(FLAG_ACTIVITY_SENDER | FLAG_BROADCAST_SENDER | FLAG_SERVICE_SENDER));
     }
 
-    @Test
-    public void onOpChanged_permissionRevoked_cancelsAllNotificationsFromPackage()
-            throws RemoteException {
-        // Have preexisting posted notifications from revoked package and other packages.
-        mService.addNotification(new NotificationRecord(mContext,
-                generateSbn("revoked", 1001, 1, 0), mTestNotificationChannel));
-        mService.addNotification(new NotificationRecord(mContext,
-                generateSbn("other", 1002, 2, 0), mTestNotificationChannel));
-        // Have preexisting enqueued notifications from revoked package and other packages.
-        mService.addEnqueuedNotification(new NotificationRecord(mContext,
-                generateSbn("revoked", 1001, 3, 0), mTestNotificationChannel));
-        mService.addEnqueuedNotification(new NotificationRecord(mContext,
-                generateSbn("other", 1002, 4, 0), mTestNotificationChannel));
-        assertThat(mService.mNotificationList).hasSize(2);
-        assertThat(mService.mEnqueuedNotifications).hasSize(2);
-
-        when(mPackageManagerInternal.getPackageUid("revoked", 0, 0)).thenReturn(1001);
-        when(mPermissionHelper.hasPermission(eq(1001))).thenReturn(false);
-
-        mOnPermissionChangeListener.onOpChanged(
-                AppOpsManager.OPSTR_POST_NOTIFICATION, "revoked", 0);
-        waitForIdle();
-
-        assertThat(mService.mNotificationList).hasSize(1);
-        assertThat(mService.mNotificationList.get(0).getSbn().getPackageName()).isEqualTo("other");
-        assertThat(mService.mEnqueuedNotifications).hasSize(1);
-        assertThat(mService.mEnqueuedNotifications.get(0).getSbn().getPackageName()).isEqualTo(
-                "other");
-    }
-
-    @Test
-    public void onOpChanged_permissionStillGranted_notificationsAreNotAffected()
-            throws RemoteException {
-        // NOTE: This combination (receiving the onOpChanged broadcast for a package, the permission
-        // being now granted, AND having previously posted notifications from said package) should
-        // never happen (if we trust the broadcasts are correct). So this test is for a what-if
-        // scenario, to verify we still handle it reasonably.
-
-        // Have preexisting posted notifications from specific package and other packages.
-        mService.addNotification(new NotificationRecord(mContext,
-                generateSbn("granted", 1001, 1, 0), mTestNotificationChannel));
-        mService.addNotification(new NotificationRecord(mContext,
-                generateSbn("other", 1002, 2, 0), mTestNotificationChannel));
-        // Have preexisting enqueued notifications from specific package and other packages.
-        mService.addEnqueuedNotification(new NotificationRecord(mContext,
-                generateSbn("granted", 1001, 3, 0), mTestNotificationChannel));
-        mService.addEnqueuedNotification(new NotificationRecord(mContext,
-                generateSbn("other", 1002, 4, 0), mTestNotificationChannel));
-        assertThat(mService.mNotificationList).hasSize(2);
-        assertThat(mService.mEnqueuedNotifications).hasSize(2);
-
-        when(mPackageManagerInternal.getPackageUid("granted", 0, 0)).thenReturn(1001);
-        when(mPermissionHelper.hasPermission(eq(1001))).thenReturn(true);
-
-        mOnPermissionChangeListener.onOpChanged(
-                AppOpsManager.OPSTR_POST_NOTIFICATION, "granted", 0);
-        waitForIdle();
-
-        assertThat(mService.mNotificationList).hasSize(2);
-        assertThat(mService.mEnqueuedNotifications).hasSize(2);
-    }
-
-    @Test
-    public void onOpChanged_permissionGranted_notifiesAppUnblocked() throws Exception {
-        when(mPackageManagerInternal.getPackageUid(PKG, 0, 0)).thenReturn(1001);
-        when(mPermissionHelper.hasPermission(eq(1001))).thenReturn(true);
-
-        mOnPermissionChangeListener.onOpChanged(
-                AppOpsManager.OPSTR_POST_NOTIFICATION, PKG, 0);
-        waitForIdle();
-        mTestableLooper.moveTimeForward(500);
-        waitForIdle();
-
-        ArgumentCaptor<Intent> captor = ArgumentCaptor.forClass(Intent.class);
-        verify(mContext).sendBroadcastAsUser(captor.capture(), any(), eq(null));
-        assertThat(captor.getValue().getAction()).isEqualTo(
-                NotificationManager.ACTION_APP_BLOCK_STATE_CHANGED);
-        assertThat(captor.getValue().getPackage()).isEqualTo(PKG);
-        assertThat(captor.getValue().getBooleanExtra(EXTRA_BLOCKED_STATE, true)).isFalse();
-    }
-
-    @Test
-    public void onOpChanged_permissionRevoked_notifiesAppBlocked() throws Exception {
-        when(mPackageManagerInternal.getPackageUid(PKG, 0, 0)).thenReturn(1001);
-        when(mPermissionHelper.hasPermission(eq(1001))).thenReturn(false);
-
-        mOnPermissionChangeListener.onOpChanged(
-                AppOpsManager.OPSTR_POST_NOTIFICATION, PKG, 0);
-        waitForIdle();
-        mTestableLooper.moveTimeForward(500);
-        waitForIdle();
-
-        ArgumentCaptor<Intent> captor = ArgumentCaptor.forClass(Intent.class);
-        verify(mContext).sendBroadcastAsUser(captor.capture(), any(), eq(null));
-        assertThat(captor.getValue().getAction()).isEqualTo(
-                NotificationManager.ACTION_APP_BLOCK_STATE_CHANGED);
-        assertThat(captor.getValue().getPackage()).isEqualTo(PKG);
-        assertThat(captor.getValue().getBooleanExtra(EXTRA_BLOCKED_STATE, false)).isTrue();
-    }
-
-    @Test
-    public void setNotificationsEnabledForPackage_disabling_clearsNotifications() throws Exception {
-        mService.addNotification(new NotificationRecord(mContext,
-                generateSbn("package", 1001, 1, 0), mTestNotificationChannel));
-        assertThat(mService.mNotificationList).hasSize(1);
-        when(mPackageManagerInternal.getPackageUid("package", 0, 0)).thenReturn(1001);
-        when(mPermissionHelper.hasRequestedPermission(any(), eq("package"), anyInt())).thenReturn(
-                true);
-
-        // Start with granted permission and simulate effect of revoking it.
-        when(mPermissionHelper.hasPermission(1001)).thenReturn(true);
-        doAnswer(invocation -> {
-            when(mPermissionHelper.hasPermission(1001)).thenReturn(false);
-            mOnPermissionChangeListener.onOpChanged(
-                    AppOpsManager.OPSTR_POST_NOTIFICATION, "package", 0);
-            return null;
-        }).when(mPermissionHelper).setNotificationPermission("package", 0, false, true);
-
-        mBinderService.setNotificationsEnabledForPackage("package", 1001, false);
-        waitForIdle();
-
-        assertThat(mService.mNotificationList).hasSize(0);
-
-        mTestableLooper.moveTimeForward(500);
-        waitForIdle();
-        verify(mContext).sendBroadcastAsUser(any(), eq(UserHandle.of(0)), eq(null));
-    }
-
     private static <T extends Parcelable> T parcelAndUnparcel(T source,
             Parcelable.Creator<T> creator) {
         Parcel parcel = Parcel.obtain();