From 2e8b4013072f30ee96f77cd691a52a98240c1b71 Mon Sep 17 00:00:00 2001
From: Kevin Han <kevhan@google.com>
Date: Thu, 11 Nov 2021 14:29:35 -0800
Subject: [PATCH] Add hibernation NPE check and unhibernate only if hibernating

Rather than depend strictly on "leaving" force-stop (i.e. stopped to not
stopped), we unhibernate when setting stopped to false and the app is
hibernating. This prevents situations where an app is not stopped but
hibernating and can never exit hibernation because the app never
"leaves" force-stop.

With this, we can add the NPE check without risk of putting the app into
a permanent hibernation state.

Bug: 205774141
Test: atest AppHibernationIntegrationTest
Test: atest PackageManagerServiceHibernationTests
Change-Id: Ia45d0c9236474841d0ba63b5265b3b835cf9f6f5
---
 .../server/pm/PackageManagerService.java      | 14 ++++----
 .../PackageManagerServiceHibernationTests.kt  | 32 +++++++++++++++++--
 2 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 5f56d4e100369..79684a8d3dae8 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -6789,24 +6789,24 @@ public class PackageManagerService extends IPackageManager.Stub
         }
         enforceCrossUserPermission(callingUid, userId, true /* requireFullPermission */,
                 true /* checkShell */, "stop package");
-        boolean shouldUnhibernate = false;
         // writer
         synchronized (mLock) {
             final PackageSetting ps = mSettings.getPackageLPr(packageName);
-            if (ps != null && ps.getStopped(userId) && !stopped) {
-                shouldUnhibernate = true;
-            }
             if (!shouldFilterApplication(ps, callingUid, userId)
                     && mSettings.setPackageStoppedStateLPw(this, packageName, stopped, userId)) {
                 scheduleWritePackageRestrictionsLocked(userId);
             }
         }
-        if (shouldUnhibernate) {
+        // If this would cause the app to leave force-stop, then also make sure to unhibernate the
+        // app if needed.
+        if (!stopped) {
             mHandler.post(() -> {
                 AppHibernationManagerInternal ah =
                         mInjector.getLocalService(AppHibernationManagerInternal.class);
-                ah.setHibernatingForUser(packageName, userId, false);
-                ah.setHibernatingGlobally(packageName, false);
+                if (ah != null && ah.isHibernatingForUser(packageName, userId)) {
+                    ah.setHibernatingForUser(packageName, userId, false);
+                    ah.setHibernatingGlobally(packageName, false);
+                }
             });
         }
     }
diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt b/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt
index bbfa4610feeaa..cfc81e684a7f6 100644
--- a/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt
+++ b/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt
@@ -29,6 +29,7 @@ import com.android.server.apphibernation.AppHibernationManagerInternal
 import com.android.server.apphibernation.AppHibernationService
 import com.android.server.extendedtestutils.wheneverStatic
 import com.android.server.testutils.whenever
+import org.junit.Assert
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
 import org.junit.Before
@@ -36,7 +37,6 @@ import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mock
-import org.mockito.Mockito
 import org.mockito.Mockito.verify
 import org.mockito.MockitoAnnotations
 
@@ -102,9 +102,10 @@ class PackageManagerServiceHibernationTests {
         val pm = createPackageManagerService()
         rule.system().validateFinalState()
 
-        pm.setPackageStoppedState(TEST_PACKAGE_NAME, true, TEST_USER_ID)
         TestableLooper.get(this).processAllMessages()
-        Mockito.clearInvocations(appHibernationManager)
+
+        whenever(appHibernationManager.isHibernatingForUser(TEST_PACKAGE_NAME, TEST_USER_ID))
+            .thenReturn(true)
 
         pm.setPackageStoppedState(TEST_PACKAGE_NAME, false, TEST_USER_ID)
 
@@ -114,6 +115,31 @@ class PackageManagerServiceHibernationTests {
         verify(appHibernationManager).setHibernatingGlobally(TEST_PACKAGE_NAME, false)
     }
 
+    @Test
+    fun testExitForceStop_nonExistingAppHibernationManager_doesNotThrowException() {
+        whenever(rule.mocks().injector.getLocalService(AppHibernationManagerInternal::class.java))
+            .thenReturn(null)
+
+        rule.system().stageScanExistingPackage(
+            TEST_PACKAGE_NAME,
+            1L,
+            rule.system().dataAppDirectory)
+        val pm = createPackageManagerService()
+        rule.system().validateFinalState()
+
+        TestableLooper.get(this).processAllMessages()
+
+        whenever(appHibernationManager.isHibernatingForUser(TEST_PACKAGE_NAME, TEST_USER_ID))
+            .thenReturn(true)
+
+        try {
+            pm.setPackageStoppedState(TEST_PACKAGE_NAME, false, TEST_USER_ID)
+            TestableLooper.get(this).processAllMessages()
+        } catch (e: Exception) {
+            Assert.fail("Method throws exception when AppHibernationManager is not ready.\n$e")
+        }
+    }
+
     @Test
     fun testGetOptimizablePackages_ExcludesGloballyHibernatingPackages() {
         rule.system().stageScanExistingPackage(