From f3ba6a641db5f812d9e392697218254052a7f6ad Mon Sep 17 00:00:00 2001
From: Riddle Hsu <riddlehsu@google.com>
Date: Fri, 16 Oct 2020 22:01:35 +0800
Subject: [PATCH] Do not update lru list for dead process

There were multiple wrong states yield a no harm result:
The updateLruProcessLocked only adds the given process to lru list
if the process doesn't have activity. In R it was no problem because
the activities are not detached from process when handling crash,
updateProcessInfo is not called from ActivityRecord#setState with
DESTROYED state. Commit 7fce339 fixed the case but introduced another
bug that WindowProcessController#handleAppDied adds the inactive
activity list to the active list that causes hasActivities returns
true, so it is still no harm because update-lru will ignore the
process with activity.

And then commit 8f7dd59 corrects the inconsistent state accidentally
by the added field mHasActivities. That corrects the return value of
hasActivitiesOrRecentTasks. So updateLruProcessLocked takes effect
to add the dead process into to lru list, which leads to the method
getPackageProcessState gets old process state from a dead process
record (pid=0).

Fixes: 170487962
Test: OneTimePermissionTest#testStickyServiceMaintainsPermissionOnRestart
Change-Id: I4449a5581cac678fd362cc22ad2754f0aada5482
---
 .../com/android/server/am/ProcessRecord.java  |  6 ++++++
 .../server/wm/WindowProcessController.java    | 19 +++++++++++++------
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/services/core/java/com/android/server/am/ProcessRecord.java b/services/core/java/com/android/server/am/ProcessRecord.java
index 2571205bb3afa..53c6758585cf5 100644
--- a/services/core/java/com/android/server/am/ProcessRecord.java
+++ b/services/core/java/com/android/server/am/ProcessRecord.java
@@ -1460,6 +1460,12 @@ class ProcessRecord implements WindowProcessListener {
             if (updateServiceConnectionActivities) {
                 mService.mServices.updateServiceConnectionActivitiesLocked(this);
             }
+            if (thread == null) {
+                // Only update lru and oom-adj if the process is alive. Because it may be called
+                // when cleaning up the last activity from handling process died, the dead process
+                // should not be added to lru list again.
+                return;
+            }
             mService.mProcessList.updateLruProcessLocked(this, activityChange, null /* client */);
             if (updateOomAdj) {
                 mService.updateOomAdjLocked(this, OomAdjuster.OOM_ADJ_REASON_ACTIVITY);
diff --git a/services/core/java/com/android/server/wm/WindowProcessController.java b/services/core/java/com/android/server/wm/WindowProcessController.java
index e8a7a9c1df92b..2e7905c64049e 100644
--- a/services/core/java/com/android/server/wm/WindowProcessController.java
+++ b/services/core/java/com/android/server/wm/WindowProcessController.java
@@ -1257,20 +1257,27 @@ public class WindowProcessController extends ConfigurationContainer<Configuratio
         mAtm.mStackSupervisor.removeHistoryRecords(this);
 
         boolean hasVisibleActivities = false;
-        if (mInactiveActivities != null && !mInactiveActivities.isEmpty()) {
+        final boolean hasInactiveActivities =
+                mInactiveActivities != null && !mInactiveActivities.isEmpty();
+        final ArrayList<ActivityRecord> activities =
+                (mHasActivities || hasInactiveActivities) ? new ArrayList<>() : mActivities;
+        if (mHasActivities) {
+            activities.addAll(mActivities);
+        }
+        if (hasInactiveActivities) {
             // Make sure that all activities in this process are handled.
-            mActivities.addAll(mInactiveActivities);
+            activities.addAll(mInactiveActivities);
         }
         if (isRemoved()) {
             // The package of the died process should be force-stopped, so make its activities as
             // finishing to prevent the process from being started again if the next top (or being
             // visible) activity also resides in the same process. This must be done before removal.
-            for (int i = mActivities.size() - 1; i >= 0; i--) {
-                mActivities.get(i).makeFinishingLocked();
+            for (int i = activities.size() - 1; i >= 0; i--) {
+                activities.get(i).makeFinishingLocked();
             }
         }
-        for (int i = mActivities.size() - 1; i >= 0; i--) {
-            final ActivityRecord r = mActivities.get(i);
+        for (int i = activities.size() - 1; i >= 0; i--) {
+            final ActivityRecord r = activities.get(i);
             if (r.mVisibleRequested || r.isVisible()) {
                 // While an activity launches a new activity, it's possible that the old activity
                 // is already requested to be hidden (mVisibleRequested=false), but this visibility