From c40fd129591953bb84920c2dbbeb93057432d2b2 Mon Sep 17 00:00:00 2001 From: Suprabh Shukla Date: Wed, 30 Oct 2024 18:33:41 -0700 Subject: Consistently use CPU uptime for temporary allowlist The handler message to evaluate the timeout is delayed based on CPU uptime. The calculation of the time throughout activity manager should just use the same clock for ending the temporary allowlist for each app. Flag: com.android.server.deviceidle.use_cpu_time_for_temp_allowlist Test: atest FrameworksMockingServicesTests:DeviceIdleControllerTest Test: Manually inspect dumps BYPASS_INCLUSIVE_LANGUAGE_REASON=Existing APIs Bug: 376561328 Change-Id: I8c256e0a5536fe8278b1021f389d7ee37b88e22c --- .../service/aconfig/device_idle.aconfig | 10 ++++ .../com/android/server/DeviceIdleController.java | 32 +++++++++-- .../android/server/am/ActivityManagerService.java | 6 +- .../com/android/server/am/FgsTempAllowList.java | 17 ++++-- services/tests/mockingservicestests/Android.bp | 4 +- .../android/server/DeviceIdleControllerTest.java | 66 +++++++++++++++++++++- 6 files changed, 118 insertions(+), 17 deletions(-) diff --git a/apex/jobscheduler/service/aconfig/device_idle.aconfig b/apex/jobscheduler/service/aconfig/device_idle.aconfig index c4d0d1850a18..426031fbeb9c 100644 --- a/apex/jobscheduler/service/aconfig/device_idle.aconfig +++ b/apex/jobscheduler/service/aconfig/device_idle.aconfig @@ -17,3 +17,13 @@ flag { description: "Disable wakelocks for background apps while Light Device Idle is active" bug: "326607666" } + +flag { + name: "use_cpu_time_for_temp_allowlist" + namespace: "backstage_power" + description: "Use CPU time for temporary allowlists" + bug: "376561328" + metadata { + purpose: PURPOSE_BUGFIX + } +} diff --git a/apex/jobscheduler/service/java/com/android/server/DeviceIdleController.java b/apex/jobscheduler/service/java/com/android/server/DeviceIdleController.java index 3e650da2e66f..41fd4a29cfd1 100644 --- a/apex/jobscheduler/service/java/com/android/server/DeviceIdleController.java +++ b/apex/jobscheduler/service/java/com/android/server/DeviceIdleController.java @@ -620,8 +620,8 @@ public class DeviceIdleController extends SystemService * the network and acquire wakelocks. Times are in milliseconds. */ @GuardedBy("this") - private final SparseArray> mTempWhitelistAppIdEndTimes - = new SparseArray<>(); + @VisibleForTesting + final SparseArray> mTempWhitelistAppIdEndTimes = new SparseArray<>(); private NetworkPolicyManagerInternal mNetworkPolicyManagerInternal; @@ -1941,7 +1941,8 @@ public class DeviceIdleController extends SystemService private static final int MSG_REPORT_IDLE_ON_LIGHT = 3; private static final int MSG_REPORT_IDLE_OFF = 4; private static final int MSG_REPORT_ACTIVE = 5; - private static final int MSG_TEMP_APP_WHITELIST_TIMEOUT = 6; + @VisibleForTesting + static final int MSG_TEMP_APP_WHITELIST_TIMEOUT = 6; @VisibleForTesting static final int MSG_REPORT_STATIONARY_STATUS = 7; private static final int MSG_FINISH_IDLE_OP = 8; @@ -2511,6 +2512,11 @@ public class DeviceIdleController extends SystemService return SystemClock.elapsedRealtime(); } + /** Returns the current elapsed realtime in milliseconds. */ + long getUptimeMillis() { + return SystemClock.uptimeMillis(); + } + LocationManager getLocationManager() { if (mLocationManager == null) { mLocationManager = mContext.getSystemService(LocationManager.class); @@ -3264,7 +3270,8 @@ public class DeviceIdleController extends SystemService void addPowerSaveTempWhitelistAppDirectInternal(int callingUid, int uid, long duration, @TempAllowListType int tempAllowListType, boolean sync, @ReasonCode int reasonCode, @Nullable String reason) { - final long timeNow = SystemClock.elapsedRealtime(); + final long timeNow = Flags.useCpuTimeForTempAllowlist() ? mInjector.getUptimeMillis() + : mInjector.getElapsedRealtime(); boolean informWhitelistChanged = false; int appId = UserHandle.getAppId(uid); synchronized (this) { @@ -3350,7 +3357,8 @@ public class DeviceIdleController extends SystemService } void checkTempAppWhitelistTimeout(int uid) { - final long timeNow = SystemClock.elapsedRealtime(); + final long timeNow = Flags.useCpuTimeForTempAllowlist() ? mInjector.getUptimeMillis() + : mInjector.getElapsedRealtime(); final int appId = UserHandle.getAppId(uid); if (DEBUG) { Slog.d(TAG, "checkTempAppWhitelistTimeout: uid=" + uid + ", timeNow=" + timeNow); @@ -5219,6 +5227,17 @@ public class DeviceIdleController extends SystemService } } + pw.println(" Flags:"); + pw.print(" "); + pw.print(Flags.FLAG_USE_CPU_TIME_FOR_TEMP_ALLOWLIST); + pw.print("="); + pw.println(Flags.useCpuTimeForTempAllowlist()); + pw.print(" "); + pw.print(Flags.FLAG_REMOVE_IDLE_LOCATION); + pw.print("="); + pw.println(Flags.removeIdleLocation()); + pw.println(); + synchronized (this) { mConstants.dump(pw); @@ -5449,7 +5468,8 @@ public class DeviceIdleController extends SystemService pw.println(" Temp whitelist schedule:"); prefix = " "; } - final long timeNow = SystemClock.elapsedRealtime(); + final long timeNow = Flags.useCpuTimeForTempAllowlist() ? mInjector.getUptimeMillis() + : mInjector.getElapsedRealtime(); for (int i = 0; i < size; i++) { pw.print(prefix); pw.print("UID="); diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 3e7bcb81c47f..6a9bc5e5665b 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -11337,7 +11337,9 @@ public class ActivityManagerService extends IActivityManager.Stub } pw.println(" mFgsStartTempAllowList:"); final long currentTimeNow = System.currentTimeMillis(); - final long elapsedRealtimeNow = SystemClock.elapsedRealtime(); + final long tempAllowlistCurrentTime = + com.android.server.deviceidle.Flags.useCpuTimeForTempAllowlist() + ? SystemClock.uptimeMillis() : SystemClock.elapsedRealtime(); mFgsStartTempAllowList.forEach((uid, entry) -> { pw.print(" " + UserHandle.formatUid(uid) + ": "); entry.second.dump(pw); @@ -11345,7 +11347,7 @@ public class ActivityManagerService extends IActivityManager.Stub // Convert entry.mExpirationTime, which is an elapsed time since boot, // to a time since epoch (i.e. System.currentTimeMillis()-based time.) final long expirationInCurrentTime = - currentTimeNow - elapsedRealtimeNow + entry.first; + currentTimeNow - tempAllowlistCurrentTime + entry.first; TimeUtils.dumpTimeWithDelta(pw, expirationInCurrentTime, currentTimeNow); pw.println(); }); diff --git a/services/core/java/com/android/server/am/FgsTempAllowList.java b/services/core/java/com/android/server/am/FgsTempAllowList.java index c28655655765..5569b6db3285 100644 --- a/services/core/java/com/android/server/am/FgsTempAllowList.java +++ b/services/core/java/com/android/server/am/FgsTempAllowList.java @@ -29,7 +29,7 @@ import java.util.function.BiConsumer; /** * List of keys that have expiration time. - * If the expiration time is less than current elapsedRealtime, the key has expired. + * If the expiration time is less than current uptime, the key has expired. * Otherwise it is valid (or allowed). * *

This is used for both FGS-BG-start restriction, and FGS-while-in-use permissions check.

@@ -42,7 +42,7 @@ public class FgsTempAllowList { private static final int DEFAULT_MAX_SIZE = 100; /** - * The value is Pair type, Pair.first is the expirationTime(an elapsedRealtime), + * The value is Pair type, Pair.first is the expirationTime(in cpu uptime), * Pair.second is the optional information entry about this key. */ private final SparseArray> mTempAllowList = new SparseArray<>(); @@ -82,7 +82,9 @@ public class FgsTempAllowList { } // The temp allowlist should be a short list with only a few entries in it. // for a very large list, HashMap structure should be used. - final long now = SystemClock.elapsedRealtime(); + final long now = com.android.server.deviceidle.Flags.useCpuTimeForTempAllowlist() + ? SystemClock.uptimeMillis() + : SystemClock.elapsedRealtime(); final int size = mTempAllowList.size(); if (size > mMaxSize) { Slog.w(TAG_AM, "FgsTempAllowList length:" + size + " exceeds maxSize" @@ -112,12 +114,15 @@ public class FgsTempAllowList { final int index = mTempAllowList.indexOfKey(uid); if (index < 0) { return null; - } else if (mTempAllowList.valueAt(index).first < SystemClock.elapsedRealtime()) { + } + final long timeNow = com.android.server.deviceidle.Flags.useCpuTimeForTempAllowlist() + ? SystemClock.uptimeMillis() + : SystemClock.elapsedRealtime(); + if (mTempAllowList.valueAt(index).first < timeNow) { mTempAllowList.removeAt(index); return null; - } else { - return mTempAllowList.valueAt(index); } + return mTempAllowList.valueAt(index); } } diff --git a/services/tests/mockingservicestests/Android.bp b/services/tests/mockingservicestests/Android.bp index c81d6be43223..9a300fb3c9fd 100644 --- a/services/tests/mockingservicestests/Android.bp +++ b/services/tests/mockingservicestests/Android.bp @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. java_defaults { - name: "FrameworkMockingServicesTests-jni-defaults", + name: "FrameworksMockingServicesTests-jni-defaults", jni_libs: [ "libmockingservicestestjni", ], @@ -30,7 +30,7 @@ package { android_test { name: "FrameworksMockingServicesTests", defaults: [ - "FrameworkMockingServicesTests-jni-defaults", + "FrameworksMockingServicesTests-jni-defaults", "modules-utils-testable-device-config-defaults", ], diff --git a/services/tests/mockingservicestests/src/com/android/server/DeviceIdleControllerTest.java b/services/tests/mockingservicestests/src/com/android/server/DeviceIdleControllerTest.java index cbc8538cf9fb..37d1c30f76f5 100644 --- a/services/tests/mockingservicestests/src/com/android/server/DeviceIdleControllerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/DeviceIdleControllerTest.java @@ -15,7 +15,12 @@ */ package com.android.server; +import static android.os.PowerExemptionManager.REASON_OTHER; +import static android.os.PowerExemptionManager.TEMPORARY_ALLOW_LIST_TYPE_FOREGROUND_SERVICE_ALLOWED; +import static android.platform.test.flag.junit.SetFlagsRule.DefaultInitValueType.NULL_DEFAULT; + import static androidx.test.InstrumentationRegistry.getContext; + import static com.android.dx.mockito.inline.extended.ExtendedMockito.doAnswer; import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing; import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; @@ -31,6 +36,7 @@ import static com.android.server.DeviceIdleController.LIGHT_STATE_INACTIVE; import static com.android.server.DeviceIdleController.LIGHT_STATE_OVERRIDE; import static com.android.server.DeviceIdleController.LIGHT_STATE_WAITING_FOR_NETWORK; import static com.android.server.DeviceIdleController.MSG_REPORT_STATIONARY_STATUS; +import static com.android.server.DeviceIdleController.MSG_TEMP_APP_WHITELIST_TIMEOUT; import static com.android.server.DeviceIdleController.STATE_ACTIVE; import static com.android.server.DeviceIdleController.STATE_IDLE; import static com.android.server.DeviceIdleController.STATE_IDLE_MAINTENANCE; @@ -41,6 +47,7 @@ import static com.android.server.DeviceIdleController.STATE_QUICK_DOZE_DELAY; import static com.android.server.DeviceIdleController.STATE_SENSING; import static com.android.server.DeviceIdleController.lightStateToString; import static com.android.server.DeviceIdleController.stateToString; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -83,6 +90,8 @@ import android.os.PowerManagerInternal; import android.os.PowerSaveState; import android.os.SystemClock; import android.os.WearModeManagerInternal; +import android.platform.test.annotations.EnableFlags; +import android.platform.test.flag.junit.SetFlagsRule; import android.provider.DeviceConfig; import android.telephony.TelephonyCallback; import android.telephony.TelephonyManager; @@ -90,12 +99,16 @@ import android.telephony.emergency.EmergencyNumber; import androidx.test.runner.AndroidJUnit4; +import com.android.internal.app.IBatteryStats; +import com.android.server.am.BatteryStatsService; import com.android.server.deviceidle.ConstraintController; +import com.android.server.deviceidle.Flags; import com.android.server.net.NetworkPolicyManagerInternal; import com.android.server.wm.ActivityTaskManagerInternal; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; @@ -115,6 +128,9 @@ import java.util.concurrent.Executor; @SuppressWarnings("GuardedBy") @RunWith(AndroidJUnit4.class) public class DeviceIdleControllerTest { + @Rule + public final SetFlagsRule mSetFlagsRule = new SetFlagsRule(NULL_DEFAULT); + private DeviceIdleController mDeviceIdleController; private DeviceIdleController.MyHandler mHandler; private AnyMotionDetectorForTest mAnyMotionDetector; @@ -157,7 +173,8 @@ public class DeviceIdleControllerTest { LocationManager locationManager; ConstraintController constraintController; // Freeze time for testing. - long nowElapsed; + volatile long nowElapsed; + volatile long nowUptime; boolean useMotionSensor = true; boolean isLocationPrefetchEnabled = true; @@ -192,6 +209,11 @@ public class DeviceIdleControllerTest { return nowElapsed; } + @Override + long getUptimeMillis() { + return nowUptime; + } + @Override LocationManager getLocationManager() { return locationManager; @@ -314,11 +336,13 @@ public class DeviceIdleControllerTest { mMockingSession = mockitoSession() .initMocks(this) .strictness(Strictness.LENIENT) + .mockStatic(BatteryStatsService.class) .spyStatic(DeviceConfig.class) .spyStatic(LocalServices.class) .startMocking(); spyOn(getContext()); doReturn(null).when(getContext()).registerReceiver(any(), any()); + doReturn(mock(IBatteryStats.class)).when(() -> BatteryStatsService.getService()); doReturn(mock(ActivityManagerInternal.class)) .when(() -> LocalServices.getService(ActivityManagerInternal.class)); doReturn(mock(ActivityTaskManagerInternal.class)) @@ -400,6 +424,46 @@ public class DeviceIdleControllerTest { LocalServices.removeServiceForTest(PowerAllowlistInternal.class); } + @Test + @EnableFlags(Flags.FLAG_USE_CPU_TIME_FOR_TEMP_ALLOWLIST) + public void testTempAllowlistCountsUptime() { + doNothing().when(getContext()).sendBroadcastAsUser(any(), any(), any(), any()); + final int testUid = 12345; + final long durationMs = 4300; + final long startTime = 100; // Arbitrary starting point in time. + mInjector.nowUptime = mInjector.nowElapsed = startTime; + + mDeviceIdleController.addPowerSaveTempWhitelistAppDirectInternal(0, testUid, durationMs, + TEMPORARY_ALLOW_LIST_TYPE_FOREGROUND_SERVICE_ALLOWED, true, REASON_OTHER, "test"); + + assertEquals(startTime + durationMs, + mDeviceIdleController.mTempWhitelistAppIdEndTimes.get(testUid).first.value); + + final InOrder inorder = inOrder(mHandler); + // mHandler is already stubbed to do nothing on handleMessage. + inorder.verify(mHandler).sendMessageDelayed( + argThat(m -> m.what == MSG_TEMP_APP_WHITELIST_TIMEOUT && m.arg1 == testUid), + eq(durationMs)); + + mInjector.nowElapsed += durationMs; + mInjector.nowUptime += 2; + // Elapsed time moved past the expiration but not uptime. The check should be rescheduled. + mDeviceIdleController.checkTempAppWhitelistTimeout(testUid); + inorder.verify(mHandler).sendMessageDelayed( + argThat(m -> m.what == MSG_TEMP_APP_WHITELIST_TIMEOUT && m.arg1 == testUid), + eq(durationMs - 2)); + assertEquals(startTime + durationMs, + mDeviceIdleController.mTempWhitelistAppIdEndTimes.get(testUid).first.value); + + mInjector.nowUptime += durationMs; + // Uptime moved past the expiration time. Uid should be removed from the temp allowlist. + mDeviceIdleController.checkTempAppWhitelistTimeout(testUid); + inorder.verify(mHandler, never()).sendMessageDelayed( + argThat(m -> m.what == MSG_TEMP_APP_WHITELIST_TIMEOUT && m.arg1 == testUid), + anyLong()); + assertFalse(mDeviceIdleController.mTempWhitelistAppIdEndTimes.contains(testUid)); + } + @Test public void testUpdateInteractivityLocked() { doReturn(false).when(mPowerManager).isInteractive(); -- cgit v1.2.3-59-g8ed1b