diff options
6 files changed, 161 insertions, 46 deletions
diff --git a/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java b/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java index 64b242334a8a..fd8ddbcf3809 100644 --- a/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java +++ b/apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java @@ -69,8 +69,8 @@ public interface JobSchedulerInternal { * @return {@code true} if the given notification channel is associated with any user-initiated * jobs. */ - boolean isNotificationChannelAssociatedWithAnyUserInitiatedJobs(String notificationChannel, - int userId, String packageName); + boolean isNotificationChannelAssociatedWithAnyUserInitiatedJobs( + @NonNull String notificationChannel, int userId, @NonNull String packageName); /** * Report a snapshot of sync-related jobs back to the sync manager diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java b/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java index 8bd3d127c21b..b9b825c9f75c 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java @@ -1925,16 +1925,14 @@ class JobConcurrencyManager { return null; } - @GuardedBy("mLock") boolean isNotificationAssociatedWithAnyUserInitiatedJobs(int notificationId, int userId, - String packageName) { + @NonNull String packageName) { return mNotificationCoordinator.isNotificationAssociatedWithAnyUserInitiatedJobs( notificationId, userId, packageName); } - @GuardedBy("mLock") - boolean isNotificationChannelAssociatedWithAnyUserInitiatedJobs(String notificationChannel, - int userId, String packageName) { + boolean isNotificationChannelAssociatedWithAnyUserInitiatedJobs( + @NonNull String notificationChannel, int userId, @NonNull String packageName) { return mNotificationCoordinator.isNotificationChannelAssociatedWithAnyUserInitiatedJobs( notificationChannel, userId, packageName); } diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobNotificationCoordinator.java b/apex/jobscheduler/service/java/com/android/server/job/JobNotificationCoordinator.java index f6e00ec24b33..d94674b5cab0 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobNotificationCoordinator.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobNotificationCoordinator.java @@ -27,9 +27,12 @@ import android.content.pm.UserPackage; import android.os.UserHandle; import android.util.ArrayMap; import android.util.ArraySet; +import android.util.IntArray; import android.util.Slog; +import android.util.SparseArrayMap; import android.util.SparseSetArray; +import com.android.internal.annotations.GuardedBy; import com.android.server.LocalServices; import com.android.server.job.controllers.JobStatus; import com.android.server.notification.NotificationManagerInternal; @@ -38,6 +41,14 @@ class JobNotificationCoordinator { private static final String TAG = "JobNotificationCoordinator"; /** + * Local lock for independent objects like mUijNotifications and mUijNotificationChannels which + * don't depend on other JS objects such as JobServiceContext which require the global JS lock. + * + * Note: do <b>NOT</b> acquire the global lock while this one is held. + */ + private final Object mUijLock = new Object(); + + /** * Mapping of UserPackage -> {notificationId -> List<JobServiceContext>} to track which jobs * are associated with each app's notifications. */ @@ -49,6 +60,27 @@ class JobNotificationCoordinator { private final ArrayMap<JobServiceContext, NotificationDetails> mNotificationDetails = new ArrayMap<>(); + /** + * Mapping of userId -> {packageName, notificationIds} tracking which notifications + * associated with each app belong to user-initiated jobs. + * + * Note: this map can be accessed without holding the main JS lock, so that other services like + * NotificationManagerService can call into JS and verify associations. + */ + @GuardedBy("mUijLock") + private final SparseArrayMap<String, IntArray> mUijNotifications = new SparseArrayMap<>(); + + /** + * Mapping of userId -> {packageName, notificationChannels} tracking which notification channels + * associated with each app are hosting a user-initiated job notification. + * + * Note: this map can be accessed without holding the main JS lock, so that other services like + * NotificationManagerService can call into JS and verify associations. + */ + @GuardedBy("mUijLock") + private final SparseArrayMap<String, ArraySet<String>> mUijNotificationChannels = + new SparseArrayMap<>(); + private static final class NotificationDetails { @NonNull public final UserPackage userPackage; @@ -81,15 +113,24 @@ class JobNotificationCoordinator { int callingPid, int callingUid, int notificationId, @NonNull Notification notification, @JobService.JobEndNotificationPolicy int jobEndNotificationPolicy) { validateNotification(packageName, callingUid, notification, jobEndNotificationPolicy); + final JobStatus jobStatus = hostingContext.getRunningJobLocked(); final NotificationDetails oldDetails = mNotificationDetails.get(hostingContext); if (oldDetails != null && oldDetails.notificationId != notificationId) { // App is switching notification IDs. Remove association with the old one. - removeNotificationAssociation(hostingContext, JobParameters.STOP_REASON_UNDEFINED); + removeNotificationAssociation(hostingContext, JobParameters.STOP_REASON_UNDEFINED, + jobStatus); } final int userId = UserHandle.getUserId(callingUid); - final JobStatus jobStatus = hostingContext.getRunningJobLocked(); if (jobStatus != null && jobStatus.startedAsUserInitiatedJob) { notification.flags |= Notification.FLAG_USER_INITIATED_JOB; + synchronized (mUijLock) { + maybeCreateUijNotificationSetsLocked(userId, packageName); + final IntArray notificationIds = mUijNotifications.get(userId, packageName); + if (notificationIds.indexOf(notificationId) == -1) { + notificationIds.add(notificationId); + } + mUijNotificationChannels.get(userId, packageName).add(notification.getChannelId()); + } } final UserPackage userPackage = UserPackage.of(userId, packageName); final NotificationDetails details = new NotificationDetails( @@ -110,7 +151,7 @@ class JobNotificationCoordinator { } void removeNotificationAssociation(@NonNull JobServiceContext hostingContext, - @JobParameters.StopReason int stopReason) { + @JobParameters.StopReason int stopReason, JobStatus completedJob) { final NotificationDetails details = mNotificationDetails.remove(hostingContext); if (details == null) { return; @@ -121,10 +162,11 @@ class JobNotificationCoordinator { Slog.wtf(TAG, "Association data structures not in sync"); return; } - final String packageName = details.userPackage.packageName; final int userId = UserHandle.getUserId(details.appUid); + final String packageName = details.userPackage.packageName; + final int notificationId = details.notificationId; boolean stripUijFlag = true; - ArraySet<JobServiceContext> associatedContexts = associations.get(details.notificationId); + ArraySet<JobServiceContext> associatedContexts = associations.get(notificationId); if (associatedContexts == null || associatedContexts.isEmpty()) { // No more jobs using this notification. Apply the final job stop policy. // If the user attempted to stop the job/app, then always remove the notification @@ -133,23 +175,50 @@ class JobNotificationCoordinator { || stopReason == JobParameters.STOP_REASON_USER) { mNotificationManagerInternal.cancelNotification( packageName, packageName, details.appUid, details.appPid, /* tag */ null, - details.notificationId, userId); + notificationId, userId); stripUijFlag = false; } } else { // Strip the UIJ flag only if there are no other UIJs associated with the notification - stripUijFlag = !isNotificationAssociatedWithAnyUserInitiatedJobs( - details.notificationId, userId, packageName); + stripUijFlag = !isNotificationUsedForAnyUij(userId, packageName, notificationId); } if (stripUijFlag) { - // Strip the user-initiated job flag from the notification. mNotificationManagerInternal.removeUserInitiatedJobFlagFromNotification( - packageName, details.notificationId, userId); + packageName, notificationId, userId); + } + + // Clean up UIJ related objects if the just completed job was a UIJ + if (completedJob != null && completedJob.startedAsUserInitiatedJob) { + maybeDeleteNotificationIdAssociation(userId, packageName, notificationId); + maybeDeleteNotificationChannelAssociation( + userId, packageName, details.notificationChannel); } } boolean isNotificationAssociatedWithAnyUserInitiatedJobs(int notificationId, - int userId, String packageName) { + int userId, @NonNull String packageName) { + synchronized (mUijLock) { + final IntArray notifications = mUijNotifications.get(userId, packageName); + if (notifications != null) { + return notifications.indexOf(notificationId) != -1; + } + return false; + } + } + + boolean isNotificationChannelAssociatedWithAnyUserInitiatedJobs( + @NonNull String notificationChannel, int userId, @NonNull String packageName) { + synchronized (mUijLock) { + final ArraySet<String> channels = mUijNotificationChannels.get(userId, packageName); + if (channels != null) { + return channels.contains(notificationChannel); + } + return false; + } + } + + private boolean isNotificationUsedForAnyUij(int userId, String packageName, + int notificationId) { final UserPackage pkgDetails = UserPackage.of(userId, packageName); final SparseSetArray<JobServiceContext> associations = mCurrentAssociations.get(pkgDetails); if (associations == null) { @@ -170,8 +239,26 @@ class JobNotificationCoordinator { return false; } - boolean isNotificationChannelAssociatedWithAnyUserInitiatedJobs(String notificationChannel, - int userId, String packageName) { + private void maybeDeleteNotificationIdAssociation(int userId, String packageName, + int notificationId) { + if (isNotificationUsedForAnyUij(userId, packageName, notificationId)) { + return; + } + + // Safe to delete - no UIJs for this package are using this notification id + synchronized (mUijLock) { + final IntArray notifications = mUijNotifications.get(userId, packageName); + if (notifications != null) { + notifications.remove(notifications.indexOf(notificationId)); + if (notifications.size() == 0) { + mUijNotifications.delete(userId, packageName); + } + } + } + } + + private void maybeDeleteNotificationChannelAssociation(int userId, String packageName, + String notificationChannel) { for (int i = mNotificationDetails.size() - 1; i >= 0; i--) { final JobServiceContext jsc = mNotificationDetails.keyAt(i); final NotificationDetails details = mNotificationDetails.get(jsc); @@ -183,11 +270,31 @@ class JobNotificationCoordinator { && details.notificationChannel.equals(notificationChannel)) { final JobStatus jobStatus = jsc.getRunningJobLocked(); if (jobStatus != null && jobStatus.startedAsUserInitiatedJob) { - return true; + return; } } } - return false; + + // Safe to delete - no UIJs for this package are using this notification channel + synchronized (mUijLock) { + ArraySet<String> channels = mUijNotificationChannels.get(userId, packageName); + if (channels != null) { + channels.remove(notificationChannel); + if (channels.isEmpty()) { + mUijNotificationChannels.delete(userId, packageName); + } + } + } + } + + @GuardedBy("mUijLock") + private void maybeCreateUijNotificationSetsLocked(int userId, String packageName) { + if (mUijNotifications.get(userId, packageName) == null) { + mUijNotifications.add(userId, packageName, new IntArray()); + } + if (mUijNotificationChannels.get(userId, packageName) == null) { + mUijNotificationChannels.add(userId, packageName, new ArraySet<>()); + } } private void validateNotification(@NonNull String packageName, int callingUid, diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java index 3aec8ba39a35..6eeff82f1158 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java @@ -3713,26 +3713,22 @@ public class JobSchedulerService extends com.android.server.SystemService @Override public boolean isNotificationAssociatedWithAnyUserInitiatedJobs(int notificationId, - int userId, String packageName) { + int userId, @NonNull String packageName) { if (packageName == null) { return false; } - synchronized (mLock) { - return mConcurrencyManager.isNotificationAssociatedWithAnyUserInitiatedJobs( - notificationId, userId, packageName); - } + return mConcurrencyManager.isNotificationAssociatedWithAnyUserInitiatedJobs( + notificationId, userId, packageName); } @Override public boolean isNotificationChannelAssociatedWithAnyUserInitiatedJobs( - String notificationChannel, int userId, String packageName) { + @NonNull String notificationChannel, int userId, @NonNull String packageName) { if (packageName == null || notificationChannel == null) { return false; } - synchronized (mLock) { - return mConcurrencyManager.isNotificationChannelAssociatedWithAnyUserInitiatedJobs( - notificationChannel, userId, packageName); - } + return mConcurrencyManager.isNotificationChannelAssociatedWithAnyUserInitiatedJobs( + notificationChannel, userId, packageName); } @Override diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java b/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java index 44700c86efef..fb36cdec490f 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java @@ -1464,7 +1464,8 @@ public final class JobServiceContext implements ServiceConnection { JobSchedulerEconomicPolicy.ACTION_JOB_TIMEOUT, String.valueOf(mRunningJob.getJobId())); } - mNotificationCoordinator.removeNotificationAssociation(this, reschedulingStopReason); + mNotificationCoordinator.removeNotificationAssociation(this, + reschedulingStopReason, completedJob); if (mWakeLock != null) { mWakeLock.release(); } diff --git a/services/tests/mockingservicestests/src/com/android/server/job/JobNotificationCoordinatorTest.java b/services/tests/mockingservicestests/src/com/android/server/job/JobNotificationCoordinatorTest.java index df2f59a1cefc..e24354f26d8b 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/JobNotificationCoordinatorTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/JobNotificationCoordinatorTest.java @@ -149,7 +149,8 @@ public class JobNotificationCoordinatorTest { .enqueueNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(), eq(notificationId), eq(notification), eq(UserHandle.getUserId(uid))); - coordinator.removeNotificationAssociation(jsc, JobParameters.STOP_REASON_UNDEFINED); + coordinator.removeNotificationAssociation(jsc, JobParameters.STOP_REASON_UNDEFINED, + jsc.getRunningJobLocked()); verify(mNotificationManagerInternal, never()) .cancelNotification(anyString(), anyString(), anyInt(), anyInt(), any(), anyInt(), anyInt()); @@ -170,7 +171,8 @@ public class JobNotificationCoordinatorTest { .enqueueNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(), eq(notificationId), eq(notification), eq(UserHandle.getUserId(uid))); - coordinator.removeNotificationAssociation(jsc, JobParameters.STOP_REASON_UNDEFINED); + coordinator.removeNotificationAssociation(jsc, JobParameters.STOP_REASON_UNDEFINED, + jsc.getRunningJobLocked()); verify(mNotificationManagerInternal) .cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(), eq(notificationId), eq(UserHandle.getUserId(uid))); @@ -292,7 +294,8 @@ public class JobNotificationCoordinatorTest { eq(notificationId2), eq(notification2), eq(UserHandle.getUserId(uid))); // Remove the first job. Only the first notification should be removed. - coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED); + coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED, + jsc1.getRunningJobLocked()); inOrder.verify(mNotificationManagerInternal) .cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(), eq(notificationId1), eq(UserHandle.getUserId(uid))); @@ -300,7 +303,8 @@ public class JobNotificationCoordinatorTest { .cancelNotification(anyString(), anyString(), anyInt(), anyInt(), any(), eq(notificationId2), anyInt()); - coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED); + coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED, + jsc2.getRunningJobLocked()); inOrder.verify(mNotificationManagerInternal) .cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(), eq(notificationId2), eq(UserHandle.getUserId(uid))); @@ -335,12 +339,14 @@ public class JobNotificationCoordinatorTest { eq(notificationId), eq(notification2), eq(UserHandle.getUserId(uid))); // Remove the first job. The notification shouldn't be touched because of the 2nd job. - coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED); + coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED, + jsc1.getRunningJobLocked()); inOrder.verify(mNotificationManagerInternal, never()) .cancelNotification(anyString(), anyString(), anyInt(), anyInt(), any(), anyInt(), anyInt()); - coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED); + coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED, + jsc2.getRunningJobLocked()); inOrder.verify(mNotificationManagerInternal) .cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(), eq(notificationId), eq(UserHandle.getUserId(uid))); @@ -376,7 +382,8 @@ public class JobNotificationCoordinatorTest { eq(notificationId), eq(notification2), eq(UserHandle.getUserId(uid2))); // Remove the first job. Only the first notification should be removed. - coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED); + coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED, + jsc1.getRunningJobLocked()); inOrder.verify(mNotificationManagerInternal) .cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid1), eq(pid), any(), eq(notificationId), eq(UserHandle.getUserId(uid1))); @@ -384,7 +391,8 @@ public class JobNotificationCoordinatorTest { .cancelNotification(anyString(), anyString(), eq(uid2), anyInt(), any(), anyInt(), anyInt()); - coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED); + coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED, + jsc2.getRunningJobLocked()); inOrder.verify(mNotificationManagerInternal) .cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid2), eq(pid), any(), eq(notificationId), eq(UserHandle.getUserId(uid2))); @@ -421,7 +429,8 @@ public class JobNotificationCoordinatorTest { eq(notificationId), eq(notification2), eq(UserHandle.getUserId(uid))); // Remove the first job. Only the first notification should be removed. - coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED); + coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED, + jsc1.getRunningJobLocked()); inOrder.verify(mNotificationManagerInternal) .cancelNotification(eq(pkg1), eq(pkg1), eq(uid), eq(pid), any(), eq(notificationId), eq(UserHandle.getUserId(uid))); @@ -429,7 +438,8 @@ public class JobNotificationCoordinatorTest { .cancelNotification(anyString(), anyString(), eq(uid), anyInt(), any(), anyInt(), anyInt()); - coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED); + coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED, + jsc2.getRunningJobLocked()); inOrder.verify(mNotificationManagerInternal) .cancelNotification(eq(pkg2), eq(pkg2), eq(uid), eq(pid), any(), eq(notificationId), eq(UserHandle.getUserId(uid))); @@ -450,7 +460,8 @@ public class JobNotificationCoordinatorTest { .enqueueNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(), eq(notificationId), eq(notification), eq(UserHandle.getUserId(uid))); - coordinator.removeNotificationAssociation(jsc, JobParameters.STOP_REASON_USER); + coordinator.removeNotificationAssociation(jsc, JobParameters.STOP_REASON_USER, + jsc.getRunningJobLocked()); verify(mNotificationManagerInternal) .cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(), eq(notificationId), eq(UserHandle.getUserId(uid))); @@ -485,12 +496,14 @@ public class JobNotificationCoordinatorTest { eq(notificationId), eq(notification2), eq(UserHandle.getUserId(uid))); // Remove the first job. The notification shouldn't be touched because of the 2nd job. - coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_USER); + coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_USER, + jsc1.getRunningJobLocked()); inOrder.verify(mNotificationManagerInternal, never()) .cancelNotification(anyString(), anyString(), anyInt(), anyInt(), any(), anyInt(), anyInt()); - coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_USER); + coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_USER, + jsc2.getRunningJobLocked()); inOrder.verify(mNotificationManagerInternal) .cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(), eq(notificationId), eq(UserHandle.getUserId(uid))); |