Merge "Relaxing minimum alarm window requirements" into sc-dev

3 files changed, 73 insertions, 30 deletions

diff --git a/apex/jobscheduler/framework/java/android/app/AlarmManager.java b/apex/jobscheduler/framework/java/android/app/AlarmManager.java
index 8b824e8a9a72..9fa7810ab155 100644
--- a/apex/jobscheduler/framework/java/android/app/AlarmManager.java
+++ b/apex/jobscheduler/framework/java/android/app/AlarmManager.java
@@ -532,9 +532,10 @@ public class AlarmManager {
      * modest timeliness requirements for its alarms.
      *
      * <p>
-     * Note: Starting with API {@link Build.VERSION_CODES#S}, the system will ensure that the window
-     * specified is at least a few minutes, as smaller windows are considered practically exact
-     * and should use the other APIs provided for exact alarms.
+     * Note: Starting with API {@link Build.VERSION_CODES#S}, apps should not pass in a window of
+     * less than 10 minutes. The system will try its best to accommodate smaller windows if the
+     * alarm is supposed to fire in the near future, but there are no guarantees and the app should
+     * expect any window smaller than 10 minutes to get elongated to 10 minutes.
      *
      * <p>
      * This method can also be used to achieve strict ordering guarantees among
@@ -588,9 +589,10 @@ public class AlarmManager {
      * if {@code null} is passed as the {@code targetHandler} parameter.
      *
      * <p>
-     * Note: Starting with API {@link Build.VERSION_CODES#S}, the system will ensure that the window
-     * specified is at least a few minutes, as smaller windows are considered practically exact
-     * and should use the other APIs provided for exact alarms.
+     * Note: Starting with API {@link Build.VERSION_CODES#S}, apps should not pass in a window of
+     * less than 10 minutes. The system will try its best to accommodate smaller windows if the
+     * alarm is supposed to fire in the near future, but there are no guarantees and the app should
+     * expect any window smaller than 10 minutes to get elongated to 10 minutes.
      *
      * @see #setWindow(int, long, long, PendingIntent)
      */
diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java
index 0eb260986472..5365218203d9 100644
--- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java
+++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java
@@ -530,8 +530,7 @@ public class AlarmManagerService extends SystemService {
         private static final long DEFAULT_MIN_FUTURITY = 5 * 1000;
         private static final long DEFAULT_MIN_INTERVAL = 60 * 1000;
         private static final long DEFAULT_MAX_INTERVAL = 365 * INTERVAL_DAY;
-        // TODO (b/185199076): Tune based on breakage reports.
-        private static final long DEFAULT_MIN_WINDOW = 30 * 60 * 1000;
+        private static final long DEFAULT_MIN_WINDOW = 10 * 60 * 1000;
         private static final long DEFAULT_ALLOW_WHILE_IDLE_WHITELIST_DURATION = 10 * 1000;
         private static final long DEFAULT_LISTENER_TIMEOUT = 5 * 1000;
         private static final int DEFAULT_MAX_ALARMS_PER_UID = 500;
@@ -1147,6 +1146,18 @@ public class AlarmManagerService extends SystemService {
         return when;
     }
 
+    /**
+     * This is the minimum window that can be requested for the given alarm. Windows smaller than
+     * this value will be elongated to match it.
+     * Current heuristic is similar to {@link #maxTriggerTime(long, long, long)}, the minimum
+     * allowed window is either {@link Constants#MIN_WINDOW} or 75% of the alarm's futurity,
+     * whichever is smaller.
+     */
+    long getMinimumAllowedWindow(long nowElapsed, long triggerElapsed) {
+        final long futurity = triggerElapsed - nowElapsed;
+        return Math.min((long) (futurity * 0.75), mConstants.MIN_WINDOW);
+    }
+
     // Apply a heuristic to { recurrence interval, futurity of the trigger time } to
     // calculate the end of our nominal delivery window for the alarm.
     static long maxTriggerTime(long now, long triggerAtTime, long interval) {
@@ -1833,25 +1844,6 @@ public class AlarmManagerService extends SystemService {
             }
         }
 
-        // Snap the window to reasonable limits.
-        if (windowLength > INTERVAL_DAY) {
-            Slog.w(TAG, "Window length " + windowLength
-                    + "ms suspiciously long; limiting to 1 day");
-            windowLength = INTERVAL_DAY;
-        } else if (windowLength > 0 && windowLength < mConstants.MIN_WINDOW
-                && (flags & FLAG_PRIORITIZE) == 0) {
-            if (CompatChanges.isChangeEnabled(AlarmManager.ENFORCE_MINIMUM_WINDOW_ON_INEXACT_ALARMS,
-                    callingPackage, UserHandle.getUserHandleForUid(callingUid))) {
-                Slog.w(TAG, "Window length " + windowLength + "ms too short; expanding to "
-                        + mConstants.MIN_WINDOW + "ms.");
-                windowLength = mConstants.MIN_WINDOW;
-            } else {
-                // TODO (b/185199076): Remove log once we have some data about what apps will break
-                Slog.wtf(TAG, "Short window " + windowLength + "ms specified by "
-                        + callingPackage);
-            }
-        }
-
         // Sanity check the recurrence interval.  This will catch people who supply
         // seconds when the API expects milliseconds, or apps trying shenanigans
         // around intentional period overflow, etc.
@@ -1883,7 +1875,7 @@ public class AlarmManagerService extends SystemService {
         // Try to prevent spamming by making sure apps aren't firing alarms in the immediate future
         final long minTrigger = nowElapsed
                 + (UserHandle.isCore(callingUid) ? 0L : mConstants.MIN_FUTURITY);
-        final long triggerElapsed = (nominalTrigger > minTrigger) ? nominalTrigger : minTrigger;
+        final long triggerElapsed = Math.max(minTrigger, nominalTrigger);
 
         final long maxElapsed;
         if (windowLength == 0) {
@@ -1893,6 +1885,25 @@ public class AlarmManagerService extends SystemService {
             // Fix this window in place, so that as time approaches we don't collapse it.
             windowLength = maxElapsed - triggerElapsed;
         } else {
+            // The window was explicitly requested. Snap it to allowable limits.
+            final long minAllowedWindow = getMinimumAllowedWindow(nowElapsed, triggerElapsed);
+            if (windowLength > INTERVAL_DAY) {
+                Slog.w(TAG, "Window length " + windowLength + "ms too long; limiting to 1 day");
+                windowLength = INTERVAL_DAY;
+            } else if ((flags & FLAG_PRIORITIZE) == 0 && windowLength < minAllowedWindow) {
+                // Prioritized alarms are exempt from minimum window limits.
+                if (CompatChanges.isChangeEnabled(
+                        AlarmManager.ENFORCE_MINIMUM_WINDOW_ON_INEXACT_ALARMS, callingPackage,
+                        UserHandle.getUserHandleForUid(callingUid))) {
+                    Slog.w(TAG, "Window length " + windowLength + "ms too short; expanding to "
+                            + minAllowedWindow + "ms.");
+                    windowLength = minAllowedWindow;
+                } else {
+                    // TODO (b/185199076): Remove temporary log to catch breaking apps.
+                    Slog.wtf(TAG, "Short window " + windowLength + "ms specified by "
+                            + callingPackage);
+                }
+            }
             maxElapsed = triggerElapsed + windowLength;
         }
         synchronized (mLock) {
diff --git a/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java
index 5222511cc479..d5e1cd6e0415 100644
--- a/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java
@@ -175,6 +175,7 @@ import org.mockito.stubbing.Answer;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.Random;
 import java.util.concurrent.Executor;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.LongConsumer;
@@ -2269,18 +2270,47 @@ public class AlarmManagerServiceTest {
                 () -> CompatChanges.isChangeEnabled(
                         eq(AlarmManager.ENFORCE_MINIMUM_WINDOW_ON_INEXACT_ALARMS),
                         anyString(), any(UserHandle.class)));
-        final long minWindow = 73;
+        final int minWindow = 73;
         setDeviceConfigLong(KEY_MIN_WINDOW, minWindow);
 
+        final Random random = new Random(42);
+
         // 0 is WINDOW_EXACT and < 0 is WINDOW_HEURISTIC.
         for (int window = 1; window <= minWindow; window++) {
             final PendingIntent pi = getNewMockPendingIntent();
-            setTestAlarm(ELAPSED_REALTIME, 0, window, pi, 0, 0, TEST_CALLING_UID, null);
+            final long futurity = random.nextInt(minWindow);
+
+            setTestAlarm(ELAPSED_REALTIME, mNowElapsedTest + futurity, window, pi, 0, 0,
+                    TEST_CALLING_UID, null);
+
+            final long minAllowed = (long) (futurity * 0.75); // This will always be <= minWindow.
+
+            assertEquals(1, mService.mAlarmStore.size());
+            final Alarm a = mService.mAlarmStore.remove(unused -> true).get(0);
+            assertEquals(Math.max(minAllowed, window), a.windowLength);
+        }
+
+        for (int window = 1; window <= minWindow; window++) {
+            final PendingIntent pi = getNewMockPendingIntent();
+            final long futurity = 2 * minWindow + window;  // implies (0.75 * futurity) > minWindow
+
+            setTestAlarm(ELAPSED_REALTIME, mNowElapsedTest + futurity, window, pi, 0, 0,
+                    TEST_CALLING_UID, null);
 
             assertEquals(1, mService.mAlarmStore.size());
             final Alarm a = mService.mAlarmStore.remove(unused -> true).get(0);
             assertEquals(minWindow, a.windowLength);
         }
+
+        for (int i = 0; i < 20; i++) {
+            final long window = minWindow + random.nextInt(100);
+            final PendingIntent pi = getNewMockPendingIntent();
+            setTestAlarm(ELAPSED_REALTIME, 0, window, pi, 0, 0, TEST_CALLING_UID, null);
+
+            assertEquals(1, mService.mAlarmStore.size());
+            final Alarm a = mService.mAlarmStore.remove(unused -> true).get(0);
+            assertEquals(window, a.windowLength);
+        }
     }
 
     @Test