Merge "Fail package if explicit health check does not pass"

services/core/java/com/android/server/PackageWatchdog.java

@@ -21,6 +21,7 @@ import static java.lang.annotation.RetentionPolicy.SOURCE;
 import android.annotation.IntDef;
 import android.annotation.Nullable;
 import android.content.Context;
+import android.content.pm.PackageManager;
 import android.content.pm.VersionedPackage;
 import android.os.Environment;
 import android.os.Handler;
@@ -433,7 +434,12 @@ public class PackageWatchdog {
             Iterator<ObserverInternal> it = mAllObservers.values().iterator();
             while (it.hasNext()) {
                 ObserverInternal observer = it.next();
-                if (!observer.updateMonitoringDurations(elapsedMs)) {
+                List<MonitoredPackage> failedPackages =
+                        observer.updateMonitoringDurations(elapsedMs);
+                if (!failedPackages.isEmpty()) {
+                    onExplicitHealthCheckFailed(observer, failedPackages);
+                }
+                if (observer.mPackages.isEmpty()) {
                     Slog.i(TAG, "Discarding observer " + observer.mName + ". All packages expired");
                     it.remove();
                 }
@@ -442,6 +448,32 @@ public class PackageWatchdog {
         saveToFileAsync();
     }
 
+    private void onExplicitHealthCheckFailed(ObserverInternal observer,
+            List<MonitoredPackage> failedPackages) {
+        mWorkerHandler.post(() -> {
+            synchronized (mLock) {
+                PackageHealthObserver registeredObserver = observer.mRegisteredObserver;
+                if (registeredObserver != null) {
+                    PackageManager pm = mContext.getPackageManager();
+                    for (int i = 0; i < failedPackages.size(); i++) {
+                        String packageName = failedPackages.get(i).mName;
+                        long versionCode = 0;
+                        try {
+                            versionCode = pm.getPackageInfo(
+                                    packageName, 0 /* flags */).getLongVersionCode();
+                        } catch (PackageManager.NameNotFoundException e) {
+                            Slog.w(TAG, "Explicit health check failed but could not find package "
+                                    + packageName);
+                            // TODO(b/120598832): Skip. We only continue to pass tests for now since
+                            // the tests don't install any packages
+                        }
+                        registeredObserver.execute(new VersionedPackage(packageName, versionCode));
+                    }
+                }
+            }
+        });
+    }
+
     /**
      * Loads mAllObservers from file.
      *
@@ -520,6 +552,7 @@ public class PackageWatchdog {
      */
     static class ObserverInternal {
         public final String mName;
+        //TODO(b/120598832): Add getter for mPackages
         public final ArrayMap<String, MonitoredPackage> mPackages;
         @Nullable
         public PackageHealthObserver mRegisteredObserver;
@@ -566,17 +599,17 @@ public class PackageWatchdog {
             }
         }
 
-        // TODO(b/120598832): For packages failing explicit health check, delay removal of the
-        // observer until after PackageHealthObserver#execute
         /**
          * Reduces the monitoring durations of all packages observed by this observer by
          *  {@code elapsedMs}. If any duration is less than 0, the package is removed from
          * observation.
          *
-         * @returns {@code true} if there are still packages to be observed, {@code false} otherwise
+         * @returns a {@link List} of packages that were removed from the observer without explicit
+         * health check passing, or an empty list if no package expired for which an explicit health
+         * check was still pending
          */
-        public boolean updateMonitoringDurations(long elapsedMs) {
-            List<MonitoredPackage> packages = new ArrayList<>();
+        public List<MonitoredPackage> updateMonitoringDurations(long elapsedMs) {
+            List<MonitoredPackage> removedPackages = new ArrayList<>();
             synchronized (mName) {
                 Iterator<MonitoredPackage> it = mPackages.values().iterator();
                 while (it.hasNext()) {
@@ -585,10 +618,13 @@ public class PackageWatchdog {
                     if (newDuration > 0) {
                         p.mDurationMs = newDuration;
                     } else {
+                        if (!p.mHasPassedHealthCheck) {
+                            removedPackages.add(p);
+                        }
                         it.remove();
                     }
                 }
-                return !mPackages.isEmpty();
+                return removedPackages;
             }
         }
 

tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java

@@ -31,7 +31,6 @@ import com.android.server.PackageWatchdog.PackageHealthObserver;
 import com.android.server.PackageWatchdog.PackageHealthObserverImpact;
 
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import java.io.File;
@@ -442,12 +441,9 @@ public class PackageWatchdogTest {
         assertEquals(0, observer2.mFailedPackages.size());
     }
 
-    // TODO: Unignore test after package failure is triggered on observer expiry with failing
-    // explicit health check
     /**
      * Test explicit health check status determines package failure or success on expiry
      */
-    @Ignore
     @Test
     public void testPackageFailureExplicitHealthCheck() throws Exception {
         PackageWatchdog watchdog = createWatchdog();