Tweaking some CPU_TIME capability assignments to bindings

Capability cpu_time should be propagated across all bindings from the
client.

Removing some unnecessary assignments of the capability that could
happen even when the client did not have this capability. Although
this is practically not expected to happen, this enforces correctness of
state by making failures apparent.

Also making sure we don't skip OomAdjuster updates on provider or
service connections when the cpu_time capabililty needed to be updated
for the host process.

Flag: com.android.server.am.use_cpu_time_capability

Test: atest FrameworksMockingServicesTests:ServiceBindingOomAdjPolicyTest
Test: atest FrameworksMockingServicesTests:MockingOomAdjusterTests

Bug: 370817323
Change-Id: I6915e4aed5aa02d2e2c6addad1f03a1db899b0ca

3 files changed, 151 insertions, 2 deletions

diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java
index f42641ece09b..aadf6f61956c 100644
--- a/services/core/java/com/android/server/am/OomAdjuster.java
+++ b/services/core/java/com/android/server/am/OomAdjuster.java
@@ -2840,7 +2840,6 @@ public class OomAdjuster {
                             return true;
                         }
                     }
-                    capability |= PROCESS_CAPABILITY_CPU_TIME;
                 }
                 // Not doing bind OOM management, so treat
                 // this guy more like a started service.
@@ -3089,7 +3088,6 @@ public class OomAdjuster {
                         return true;
                     }
                 }
-                capability |= PROCESS_CAPABILITY_CPU_TIME;
             }
         }
         if (cr.hasFlag(Context.BIND_TREAT_LIKE_ACTIVITY)) {
@@ -4243,6 +4241,11 @@ public class OomAdjuster {
                         != client.getSetCapability()) {
             // The connection might elevate the importance of the service's capabilities.
             needDryRun = true;
+        } else if (Flags.useCpuTimeCapability()
+                && (client.getSetCapability() & ~app.getSetCapability()
+                    & PROCESS_CAPABILITY_CPU_TIME) != 0) {
+            // The connection might grant PROCESS_CAPABILITY_CPU_TIME to the service.
+            needDryRun = true;
         } else if (Flags.unfreezeBindPolicyFix()
                 && cr.hasFlag(Context.BIND_WAIVE_PRIORITY
                             | Context.BIND_ALLOW_OOM_MANAGEMENT)) {
@@ -4290,6 +4293,10 @@ public class OomAdjuster {
                 && client.mOptRecord.shouldNotFreeze()) {
             // Process has shouldNotFreeze and it could have gotten it from the client.
             return true;
+        } else if (Flags.useCpuTimeCapability()
+                && (client.getSetCapability() & app.getSetCapability()
+                    & PROCESS_CAPABILITY_CPU_TIME) != 0) {
+            return true;
         }
         return false;
     }
@@ -4309,6 +4316,11 @@ public class OomAdjuster {
                 && client.mOptRecord.shouldNotFreeze()
                 && !app.mOptRecord.shouldNotFreeze()) {
             needDryRun = true;
+        } else if (Flags.useCpuTimeCapability()
+                && (client.getSetCapability() & ~app.getSetCapability()
+                    & PROCESS_CAPABILITY_CPU_TIME) != 0) {
+            // The connection might grant PROCESS_CAPABILITY_CPU_TIME to the provider.
+            needDryRun = true;
         }
 
         if (needDryRun) {
@@ -4335,6 +4347,10 @@ public class OomAdjuster {
                 && client.mOptRecord.shouldNotFreeze()) {
             // Process has shouldNotFreeze and it could have gotten it from the client.
             return true;
+        } else if (Flags.useCpuTimeCapability()
+                && (client.getSetCapability() & app.getSetCapability()
+                    & PROCESS_CAPABILITY_CPU_TIME) != 0) {
+            return true;
         }
 
         return false;
diff --git a/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java b/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java
index 1efe4707fc11..9e96800ca2e9 100644
--- a/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java
+++ b/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java
@@ -18,6 +18,7 @@ package com.android.server.am;
 
 import static android.app.ActivityManager.PROCESS_CAPABILITY_ALL;
 import static android.app.ActivityManager.PROCESS_CAPABILITY_BFSL;
+import static android.app.ActivityManager.PROCESS_CAPABILITY_CPU_TIME;
 import static android.app.ActivityManager.PROCESS_STATE_BOUND_FOREGROUND_SERVICE;
 import static android.app.ActivityManager.PROCESS_STATE_BOUND_TOP;
 import static android.app.ActivityManager.PROCESS_STATE_CACHED_ACTIVITY;
@@ -40,6 +41,7 @@ import static android.app.ActivityManager.PROCESS_STATE_TOP_SLEEPING;
 import static android.app.ActivityManager.PROCESS_STATE_TRANSIENT_BACKGROUND;
 import static android.app.ActivityManagerInternal.OOM_ADJ_REASON_ACTIVITY;
 import static android.app.ActivityManagerInternal.OOM_ADJ_REASON_NONE;
+import static android.app.ActivityManagerInternal.OOM_ADJ_REASON_SHORT_FGS_TIMEOUT;
 import static android.content.pm.ServiceInfo.FOREGROUND_SERVICE_TYPE_SHORT_SERVICE;
 
 import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
@@ -283,6 +285,15 @@ public class MockingOomAdjusterTests {
         }
     }
 
+    private static void assertNoCpuTime(ProcessRecord app) {
+        assertEquals(0, app.mState.getSetCapability() & PROCESS_CAPABILITY_CPU_TIME);
+    }
+
+    private static void assertCpuTime(ProcessRecord app) {
+        assertEquals(PROCESS_CAPABILITY_CPU_TIME,
+                app.mState.getSetCapability() & PROCESS_CAPABILITY_CPU_TIME);
+    }
+
     private static void assertBfsl(ProcessRecord app) {
         assertEquals(PROCESS_CAPABILITY_BFSL,
                 app.mState.getSetCapability() & PROCESS_CAPABILITY_BFSL);
@@ -661,6 +672,7 @@ public class MockingOomAdjusterTests {
         // SHORT_SERVICE, timed out already.
         s = ServiceRecord.newEmptyInstanceForTest(mService);
         s.appInfo = new ApplicationInfo();
+
         mProcessStateController.setStartRequested(s, true);
         s.isForeground = true;
         s.foregroundServiceType = FOREGROUND_SERVICE_TYPE_SHORT_SERVICE;
@@ -687,6 +699,51 @@ public class MockingOomAdjusterTests {
 
     @SuppressWarnings("GuardedBy")
     @Test
+    @EnableFlags(Flags.FLAG_USE_CPU_TIME_CAPABILITY)
+    public void testUpdateOomAdjFreezeState_bindingFromShortFgs() {
+        // Setting up a started short FGS within app1.
+        final ServiceRecord s = ServiceRecord.newEmptyInstanceForTest(mService);
+        s.appInfo = new ApplicationInfo();
+        mProcessStateController.setStartRequested(s, true);
+        s.isForeground = true;
+        s.foregroundServiceType = FOREGROUND_SERVICE_TYPE_SHORT_SERVICE;
+        mProcessStateController.setShortFgsInfo(s, SystemClock.uptimeMillis());
+
+        final ProcessRecord app = spy(makeDefaultProcessRecord(MOCKAPP_PID, MOCKAPP_UID,
+                MOCKAPP_PROCESSNAME, MOCKAPP_PACKAGENAME, true));
+        mProcessStateController.setHostProcess(s, app);
+        mProcessStateController.setHasForegroundServices(app.mServices, true,
+                FOREGROUND_SERVICE_TYPE_SHORT_SERVICE, /* hasNoneType=*/false);
+        mProcessStateController.startService(app.mServices, s);
+        app.mState.setLastTopTime(SystemClock.uptimeMillis()
+                - mService.mConstants.TOP_TO_FGS_GRACE_DURATION);
+
+        final ProcessRecord app2 = spy(makeDefaultProcessRecord(MOCKAPP2_PID, MOCKAPP2_UID,
+                MOCKAPP2_PROCESSNAME, MOCKAPP2_PACKAGENAME, false));
+        // App1 with short service binds to app2
+        bindService(app2, app, null, null, 0, mock(IBinder.class));
+
+        setProcessesToLru(app, app2);
+        updateOomAdj(app);
+
+        assertCpuTime(app);
+        assertCpuTime(app2);
+
+        // Timeout the short FGS.
+        mProcessStateController.setShortFgsInfo(s, SystemClock.uptimeMillis()
+                - mService.mConstants.mShortFgsTimeoutDuration
+                - mService.mConstants.mShortFgsProcStateExtraWaitDuration);
+        mService.mServices.onShortFgsProcstateTimeout(s);
+        // mService is a mock, but this verifies that the timeout would trigger an update.
+        verify(mService).updateOomAdjLocked(app, OOM_ADJ_REASON_SHORT_FGS_TIMEOUT);
+        updateOomAdj(app);
+
+        assertNoCpuTime(app);
+        assertNoCpuTime(app2);
+    }
+
+    @SuppressWarnings("GuardedBy")
+    @Test
     public void testUpdateOomAdj_DoOne_OverlayUi() {
         ProcessRecord app = spy(makeDefaultProcessRecord(MOCKAPP_PID, MOCKAPP_UID,
                 MOCKAPP_PROCESSNAME, MOCKAPP_PACKAGENAME, true));
@@ -3142,11 +3199,19 @@ public class MockingOomAdjusterTests {
         assertEquals(true, app.getUidRecord().isSetAllowListed());
         assertFreezeState(app, false);
         assertFreezeState(app2, false);
+        if (Flags.useCpuTimeCapability()) {
+            assertCpuTime(app);
+            assertCpuTime(app2);
+        }
 
         mProcessStateController.setUidTempAllowlistStateLSP(MOCKAPP_UID, false);
         assertEquals(false, app.getUidRecord().isSetAllowListed());
         assertFreezeState(app, true);
         assertFreezeState(app2, true);
+        if (Flags.useCpuTimeCapability()) {
+            assertNoCpuTime(app);
+            assertNoCpuTime(app2);
+        }
     }
 
     @SuppressWarnings("GuardedBy")
@@ -3171,6 +3236,11 @@ public class MockingOomAdjusterTests {
         assertFreezeState(app, false);
         assertFreezeState(app2, false);
         assertFreezeState(app3, false);
+        if (Flags.useCpuTimeCapability()) {
+            assertCpuTime(app);
+            assertCpuTime(app2);
+            assertCpuTime(app3);
+        }
 
         // Remove app1 from allowlist.
         mProcessStateController.setUidTempAllowlistStateLSP(MOCKAPP_UID, false);
@@ -3179,6 +3249,11 @@ public class MockingOomAdjusterTests {
         assertFreezeState(app, true);
         assertFreezeState(app2, false);
         assertFreezeState(app3, false);
+        if (Flags.useCpuTimeCapability()) {
+            assertNoCpuTime(app);
+            assertCpuTime(app2);
+            assertCpuTime(app3);
+        }
 
         // Now remove app2 from allowlist.
         mProcessStateController.setUidTempAllowlistStateLSP(MOCKAPP2_UID, false);
@@ -3187,6 +3262,11 @@ public class MockingOomAdjusterTests {
         assertFreezeState(app, true);
         assertFreezeState(app2, true);
         assertFreezeState(app3, true);
+        if (Flags.useCpuTimeCapability()) {
+            assertNoCpuTime(app);
+            assertNoCpuTime(app2);
+            assertNoCpuTime(app3);
+        }
     }
 
     @SuppressWarnings("GuardedBy")
diff --git a/services/tests/mockingservicestests/src/com/android/server/am/ServiceBindingOomAdjPolicyTest.java b/services/tests/mockingservicestests/src/com/android/server/am/ServiceBindingOomAdjPolicyTest.java
index 2988c77703b7..7e052dcba3fd 100644
--- a/services/tests/mockingservicestests/src/com/android/server/am/ServiceBindingOomAdjPolicyTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/am/ServiceBindingOomAdjPolicyTest.java
@@ -16,6 +16,7 @@
 
 package com.android.server.am;
 
+import static android.app.ActivityManager.PROCESS_CAPABILITY_CPU_TIME;
 import static android.app.ActivityManager.PROCESS_CAPABILITY_FOREGROUND_MICROPHONE;
 import static android.app.ActivityManager.PROCESS_CAPABILITY_NONE;
 import static android.app.ActivityManager.PROCESS_STATE_CACHED_EMPTY;
@@ -64,6 +65,8 @@ import android.content.pm.ServiceInfo;
 import android.os.Handler;
 import android.os.HandlerThread;
 import android.os.IBinder;
+import android.platform.test.annotations.DisableFlags;
+import android.platform.test.annotations.EnableFlags;
 import android.platform.test.annotations.Presubmit;
 import android.platform.test.annotations.RequiresFlagsDisabled;
 import android.platform.test.annotations.RequiresFlagsEnabled;
@@ -326,6 +329,7 @@ public final class ServiceBindingOomAdjPolicyTest {
 
     @Test
     @RequiresFlagsEnabled(com.android.server.am.Flags.FLAG_UNFREEZE_BIND_POLICY_FIX)
+    @DisableFlags(Flags.FLAG_USE_CPU_TIME_CAPABILITY)
     public void testServiceDistinctBindingOomAdjShouldNotFreeze() throws Exception {
         // Enable the flags.
         mSetFlagsRule.enableFlags(Flags.FLAG_SERVICE_BINDING_OOM_ADJ_POLICY);
@@ -418,6 +422,7 @@ public final class ServiceBindingOomAdjPolicyTest {
 
     @Test
     @RequiresFlagsEnabled(com.android.server.am.Flags.FLAG_UNFREEZE_BIND_POLICY_FIX)
+    @DisableFlags(Flags.FLAG_USE_CPU_TIME_CAPABILITY)
     public void testServiceDistinctBindingOomAdjAllowOomManagement() throws Exception {
         // Enable the flags.
         mSetFlagsRule.enableFlags(Flags.FLAG_SERVICE_BINDING_OOM_ADJ_POLICY);
@@ -497,6 +502,7 @@ public final class ServiceBindingOomAdjPolicyTest {
 
     @Test
     @RequiresFlagsEnabled(com.android.server.am.Flags.FLAG_UNFREEZE_BIND_POLICY_FIX)
+    @DisableFlags(Flags.FLAG_USE_CPU_TIME_CAPABILITY)
     public void testServiceDistinctBindingOomAdjWaivePriority_propagateUnfreeze() throws Exception {
         // Enable the flags.
         mSetFlagsRule.enableFlags(Flags.FLAG_SERVICE_BINDING_OOM_ADJ_POLICY);
@@ -574,6 +580,50 @@ public final class ServiceBindingOomAdjPolicyTest {
     }
 
     @Test
+    @RequiresFlagsEnabled({
+            Flags.FLAG_UNFREEZE_BIND_POLICY_FIX,
+            Flags.FLAG_SERVICE_BINDING_OOM_ADJ_POLICY
+    })
+    @EnableFlags(Flags.FLAG_USE_CPU_TIME_CAPABILITY)
+    public void testServiceDistinctBindingOomAdj_propagateCpuTimeCapability() throws Exception {
+        // Note that PROCESS_CAPABILITY_CPU_TIME is special and should be propagated even when
+        // BIND_INCLUDE_CAPABILITIES is not present.
+        performTestServiceDistinctBindingOomAdj(TEST_APP1_PID, TEST_APP1_UID,
+                PROCESS_STATE_HOME, HOME_APP_ADJ, PROCESS_CAPABILITY_CPU_TIME, TEST_APP1_NAME,
+                this::setHomeProcess,
+                TEST_APP2_PID, TEST_APP2_UID, PROCESS_STATE_FOREGROUND_SERVICE,
+                PERCEPTIBLE_APP_ADJ, PROCESS_CAPABILITY_NONE, TEST_APP2_NAME, TEST_SERVICE2_NAME,
+                this::setHasForegroundServices,
+                BIND_AUTO_CREATE,
+                atLeastOnce(), atLeastOnce());
+
+        // BIND_WAIVE_PRIORITY should not affect propagation of capability CPU_TIME
+        performTestServiceDistinctBindingOomAdj(TEST_APP1_PID, TEST_APP1_UID,
+                PROCESS_STATE_FOREGROUND_SERVICE, PERCEPTIBLE_APP_ADJ, PROCESS_CAPABILITY_CPU_TIME,
+                TEST_APP1_NAME,
+                this::setHasForegroundServices,
+                TEST_APP2_PID, TEST_APP2_UID, PROCESS_STATE_HOME, HOME_APP_ADJ,
+                PROCESS_CAPABILITY_NONE, TEST_APP2_NAME, TEST_SERVICE2_NAME,
+                this::setHomeProcess,
+                BIND_AUTO_CREATE | BIND_WAIVE_PRIORITY,
+                atLeastOnce(), atLeastOnce());
+
+        // If both process have the capability, the bind should not need an update but the unbind
+        // is not safe to skip.
+        // Note that this check can fail on future changes that are not related to
+        // PROCESS_CAPABILITY_CPU_TIME and trigger updates but this is important to ensure
+        // efficiency of OomAdjuster.
+        performTestServiceDistinctBindingOomAdj(TEST_APP1_PID, TEST_APP1_UID,
+                PROCESS_STATE_HOME, HOME_APP_ADJ, PROCESS_CAPABILITY_CPU_TIME, TEST_APP1_NAME,
+                this::setHomeProcess,
+                TEST_APP2_PID, TEST_APP2_UID, PROCESS_STATE_HOME, HOME_APP_ADJ,
+                PROCESS_CAPABILITY_CPU_TIME, TEST_APP2_NAME, TEST_SERVICE2_NAME,
+                this::setHomeProcess,
+                BIND_AUTO_CREATE,
+                never(), atLeastOnce());
+    }
+
+    @Test
     @RequiresFlagsDisabled(com.android.server.am.Flags.FLAG_UNFREEZE_BIND_POLICY_FIX)
     public void testServiceDistinctBindingOomAdjWaivePriority() throws Exception {
         // Enable the flags.
@@ -624,6 +674,9 @@ public final class ServiceBindingOomAdjPolicyTest {
         // Enable the flags.
         mSetFlagsRule.enableFlags(Flags.FLAG_SERVICE_BINDING_OOM_ADJ_POLICY);
 
+        // Note that some capabilities like PROCESS_CAPABILITY_CPU_TIME are special and propagated
+        // regardless of BIND_INCLUDE_CAPABILITIES. We don't test for them here.
+
         // Verify that there should be 0 oom adj update
         // because we didn't specify the "BIND_INCLUDE_CAPABILITIES"
         performTestServiceDistinctBindingOomAdj(TEST_APP1_PID, TEST_APP1_UID,