Merge "Only sync requests when there are changes to report" into rvc-dev

2 files changed, 65 insertions, 5 deletions

diff --git a/services/core/java/com/android/server/PackageWatchdog.java b/services/core/java/com/android/server/PackageWatchdog.java
index b464422e9e3d..2cfe404f8c6f 100644
--- a/services/core/java/com/android/server/PackageWatchdog.java
+++ b/services/core/java/com/android/server/PackageWatchdog.java
@@ -161,6 +161,9 @@ public class PackageWatchdog {
     private final Runnable mSaveToFile = this::saveToFile;
     private final SystemClock mSystemClock;
     private final BootThreshold mBootThreshold;
+    // The set of packages that have been synced with the ExplicitHealthCheckController
+    @GuardedBy("mLock")
+    private Set<String> mRequestedHealthCheckPackages = new ArraySet<>();
     @GuardedBy("mLock")
     private boolean mIsPackagesReady;
     // Flag to control whether explicit health checks are supported or not
@@ -624,17 +627,22 @@ public class PackageWatchdog {
      * @see #syncRequestsAsync
      */
     private void syncRequests() {
-        Set<String> packages = null;
+        boolean syncRequired = false;
         synchronized (mLock) {
             if (mIsPackagesReady) {
-                packages = getPackagesPendingHealthChecksLocked();
+                Set<String> packages = getPackagesPendingHealthChecksLocked();
+                if (!packages.equals(mRequestedHealthCheckPackages)) {
+                    syncRequired = true;
+                    mRequestedHealthCheckPackages = packages;
+                }
             } // else, we will sync requests when packages become ready
         }
 
         // Call outside lock to avoid holding lock when calling into the controller.
-        if (packages != null) {
-            Slog.i(TAG, "Syncing health check requests for packages: " + packages);
-            mHealthCheckController.syncRequests(packages);
+        if (syncRequired) {
+            Slog.i(TAG, "Syncing health check requests for packages: "
+                    + mRequestedHealthCheckPackages);
+            mHealthCheckController.syncRequests(mRequestedHealthCheckPackages);
         }
     }
 
diff --git a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
index 8cc8cf4d2a97..819fc020e397 100644
--- a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
+++ b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
@@ -1063,6 +1063,52 @@ public class PackageWatchdogTest {
         assertThat(bootObserver2.mitigatedBootLoop()).isFalse();
     }
 
+    /**
+     * Test to verify that Package Watchdog syncs health check requests with the controller
+     * correctly, and that the requests are only synced when the set of observed packages
+     * changes.
+     */
+    @Test
+    public void testSyncHealthCheckRequests() {
+        TestController testController = spy(TestController.class);
+        testController.setSupportedPackages(List.of(APP_A, APP_B, APP_C));
+        PackageWatchdog watchdog = createWatchdog(testController, true);
+
+        TestObserver testObserver1 = new TestObserver(OBSERVER_NAME_1);
+        watchdog.registerHealthObserver(testObserver1);
+        watchdog.startObservingHealth(testObserver1, List.of(APP_A), LONG_DURATION);
+        mTestLooper.dispatchAll();
+
+        TestObserver testObserver2 = new TestObserver(OBSERVER_NAME_2);
+        watchdog.registerHealthObserver(testObserver2);
+        watchdog.startObservingHealth(testObserver2, List.of(APP_B), LONG_DURATION);
+        mTestLooper.dispatchAll();
+
+        TestObserver testObserver3 = new TestObserver(OBSERVER_NAME_3);
+        watchdog.registerHealthObserver(testObserver3);
+        watchdog.startObservingHealth(testObserver3, List.of(APP_C), LONG_DURATION);
+        mTestLooper.dispatchAll();
+
+        watchdog.unregisterHealthObserver(testObserver1);
+        mTestLooper.dispatchAll();
+
+        watchdog.unregisterHealthObserver(testObserver2);
+        mTestLooper.dispatchAll();
+
+        watchdog.unregisterHealthObserver(testObserver3);
+        mTestLooper.dispatchAll();
+
+        List<Set> expectedSyncRequests = List.of(
+                Set.of(APP_A),
+                Set.of(APP_A, APP_B),
+                Set.of(APP_A, APP_B, APP_C),
+                Set.of(APP_B, APP_C),
+                Set.of(APP_C),
+                Set.of()
+        );
+        assertThat(testController.getSyncRequests()).isEqualTo(expectedSyncRequests);
+    }
+
     private void adoptShellPermissions(String... permissions) {
         InstrumentationRegistry
                 .getInstrumentation()
@@ -1219,6 +1265,7 @@ public class PackageWatchdogTest {
         private Consumer<String> mPassedConsumer;
         private Consumer<List<PackageConfig>> mSupportedConsumer;
         private Runnable mNotifySyncRunnable;
+        private List<Set> mSyncRequests = new ArrayList<>();
 
         @Override
         public void setEnabled(boolean enabled) {
@@ -1238,6 +1285,7 @@ public class PackageWatchdogTest {
 
         @Override
         public void syncRequests(Set<String> packages) {
+            mSyncRequests.add(packages);
             mRequestedPackages.clear();
             if (mIsEnabled) {
                 packages.retainAll(mSupportedPackages);
@@ -1268,6 +1316,10 @@ public class PackageWatchdogTest {
                 return Collections.emptyList();
             }
         }
+
+        public List<Set> getSyncRequests() {
+            return mSyncRequests;
+        }
     }
 
     private static class TestClock implements PackageWatchdog.SystemClock {