From 93f9b8b82e2897ed4dd5502b4c25b96e961c7389 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
---
 .../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 47e24b1cd7d83..5a543323bee38 100644
--- a/services/core/java/com/android/server/am/OomAdjuster.java
+++ b/services/core/java/com/android/server/am/OomAdjuster.java
@@ -1500,7 +1500,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;
@@ -1891,10 +1891,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();
                         }
@@ -1915,6 +1911,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 1ef9d13be8a0a..50a0a687603c8 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;
@@ -1851,6 +1852,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() {