From ef65fb8424a87630b53fbe0ba37f867ba14da3d6 Mon Sep 17 00:00:00 2001 From: Zimuzo Date: Thu, 28 Feb 2019 10:44:29 +0000 Subject: [PATCH] Fail package if explicit health check does not pass As part of extending PackageWatchdog with explicit health check support in Ib4322c327bcb00ca9a3fbdc83579e7b5f2fd633b. Trigger the observers #execute method if a package never passed explicit health check on expiry. Bug: 120598832 Test: atest PackageWatchdogTest Change-Id: I8e916a6ca115d3883fe29f66456da36cd0ed09fb --- .../com/android/server/PackageWatchdog.java | 50 ++++++++++++++++--- .../android/server/PackageWatchdogTest.java | 4 -- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/PackageWatchdog.java b/services/core/java/com/android/server/PackageWatchdog.java index 0d17c13fcae1b..660109cf61140 100644 --- a/services/core/java/com/android/server/PackageWatchdog.java +++ b/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 it = mAllObservers.values().iterator(); while (it.hasNext()) { ObserverInternal observer = it.next(); - if (!observer.updateMonitoringDurations(elapsedMs)) { + List 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 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 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 packages = new ArrayList<>(); + public List updateMonitoringDurations(long elapsedMs) { + List removedPackages = new ArrayList<>(); synchronized (mName) { Iterator 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; } } diff --git a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java index 44bd75beb143a..d0c261279c9d2 100644 --- a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java +++ b/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();