From 678b445a564d71428083e8a0bb8237e77418dfdd Mon Sep 17 00:00:00 2001
From: Jing Ji <jji@google.com>
Date: Wed, 1 Dec 2021 00:39:31 -0800
Subject: [PATCH] Fix process capabilities on re-computation for cycles in
 oomAdjuster

When retrying the oomAdj computation because of cycles, it's scanning
the list of processes in the opposite direction, we should preserve
whatever capabilities we have got before the retry.

Bug: 206585758
Test: atest MockingOomAdjusterTests
Change-Id: Ia7d0ec7a39aac5bf59ae837b4af1ed7b6512521c
Merged-In: Ia7d0ec7a39aac5bf59ae837b4af1ed7b6512521c
---
 .../com/android/server/am/OomAdjuster.java    | 10 +++---
 .../server/am/MockingOomAdjusterTests.java    | 31 +++++++++++++++++++
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java
index ff480d1f614ce..7673123c64768 100644
--- a/services/core/java/com/android/server/am/OomAdjuster.java
+++ b/services/core/java/com/android/server/am/OomAdjuster.java
@@ -1633,7 +1633,7 @@ public class OomAdjuster {
         int schedGroup;
         int procState;
         int cachedAdjSeq;
-        int capability = 0;
+        int capability = cycleReEval ? app.mState.getCurCapability() : 0;
 
         boolean foregroundActivities = false;
         boolean hasVisibleActivities = false;
@@ -2018,10 +2018,6 @@ public class OomAdjuster {
                     }
 
                     if ((cr.flags & Context.BIND_WAIVE_PRIORITY) == 0) {
-                        if (shouldSkipDueToCycle(app, cstate, procState, adj, cycleReEval)) {
-                            continue;
-                        }
-
                         if (cr.hasFlag(Context.BIND_INCLUDE_CAPABILITIES)) {
                             capability |= cstate.getCurCapability();
                         }
@@ -2042,6 +2038,10 @@ public class OomAdjuster {
                             }
                         }
 
+                        if (shouldSkipDueToCycle(app, cstate, procState, adj, cycleReEval)) {
+                            continue;
+                        }
+
                         if (clientProcState >= PROCESS_STATE_CACHED_ACTIVITY) {
                             // If the other app is cached for any reason, for purposes here
                             // we are going to consider it empty.  The specific cached state
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 18f1267b890f9..0dd4f5b338b44 100644
--- a/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java
+++ b/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java
@@ -16,6 +16,7 @@
 
 package com.android.server.am;
 
+import static android.app.ActivityManager.PROCESS_CAPABILITY_ALL;
 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;
@@ -1852,6 +1853,36 @@ public class MockingOomAdjusterTests {
                 SCHED_GROUP_DEFAULT);
     }
 
+    @SuppressWarnings("GuardedBy")
+    @Test
+    public void testUpdateOomAdj_DoAll_BoundByPersService_Cycle_Branch_Capability() {
+        ProcessRecord app = spy(makeDefaultProcessRecord(MOCKAPP_PID, MOCKAPP_UID,
+                MOCKAPP_PROCESSNAME, MOCKAPP_PACKAGENAME, false));
+        ProcessRecord client = spy(makeDefaultProcessRecord(MOCKAPP2_PID, MOCKAPP2_UID,
+                MOCKAPP2_PROCESSNAME, MOCKAPP2_PACKAGENAME, false));
+        bindService(app, client, null, Context.BIND_INCLUDE_CAPABILITIES, mock(IBinder.class));
+        ProcessRecord client2 = spy(makeDefaultProcessRecord(MOCKAPP3_PID, MOCKAPP3_UID,
+                MOCKAPP3_PROCESSNAME, MOCKAPP3_PACKAGENAME, false));
+        bindService(client, client2, null, Context.BIND_INCLUDE_CAPABILITIES, mock(IBinder.class));
+        bindService(client2, app, null, Context.BIND_INCLUDE_CAPABILITIES, mock(IBinder.class));
+        ProcessRecord client3 = spy(makeDefaultProcessRecord(MOCKAPP4_PID, MOCKAPP4_UID,
+                MOCKAPP4_PROCESSNAME, MOCKAPP4_PACKAGENAME, false));
+        client3.mState.setMaxAdj(PERSISTENT_PROC_ADJ);
+        bindService(app, client3, null, Context.BIND_INCLUDE_CAPABILITIES, mock(IBinder.class));
+        ArrayList<ProcessRecord> lru = sService.mProcessList.getLruProcessesLOSP();
+        lru.clear();
+        lru.add(app);
+        lru.add(client);
+        lru.add(client2);
+        lru.add(client3);
+        sService.mWakefulness.set(PowerManagerInternal.WAKEFULNESS_AWAKE);
+        sService.mOomAdjuster.updateOomAdjLocked(OomAdjuster.OOM_ADJ_REASON_NONE);
+
+        assertEquals(PROCESS_CAPABILITY_ALL, client.mState.getSetCapability());
+        assertEquals(PROCESS_CAPABILITY_ALL, client2.mState.getSetCapability());
+        assertEquals(PROCESS_CAPABILITY_ALL, app.mState.getSetCapability());
+    }
+
     @SuppressWarnings("GuardedBy")
     @Test
     public void testUpdateOomAdj_DoAll_Provider_Cycle_Branch_2() {