Add PackageHealthObserverImpact

When a package fails health check, observers will report the impact of their
action on the user. Only the observer with the least user impact will be
allowed to take action.

Bug: 120598832
Test: atest PackageWatchdogTest
Change-Id: I15f358cd599431e1d7ea211aea5b1391f4aa33ab

3 files changed, 312 insertions, 110 deletions

diff --git a/services/core/java/com/android/server/PackageWatchdog.java b/services/core/java/com/android/server/PackageWatchdog.java
index 8adc416fda95..84577f125f1d 100644
--- a/services/core/java/com/android/server/PackageWatchdog.java
+++ b/services/core/java/com/android/server/PackageWatchdog.java
@@ -16,6 +16,9 @@
 
 package com.android.server;
 
+import static java.lang.annotation.RetentionPolicy.SOURCE;
+
+import android.annotation.IntDef;
 import android.annotation.Nullable;
 import android.content.Context;
 import android.os.Environment;
@@ -46,6 +49,7 @@ import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.annotation.Retention;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -55,7 +59,8 @@ import java.util.Set;
 
 /**
  * Monitors the health of packages on the system and notifies interested observers when packages
- * fail. All registered observers will be notified until an observer takes a mitigation action.
+ * fail. On failure, the registered observer with the least user impacting mitigation will
+ * be notified.
  */
 public class PackageWatchdog {
     private static final String TAG = "PackageWatchdog";
@@ -78,7 +83,8 @@ public class PackageWatchdog {
     private final Context mContext;
     // Handler to run package cleanup runnables
     private final Handler mTimerHandler;
-    private final Handler mIoHandler;
+    // Handler for processing IO and observer actions
+    private final Handler mWorkerHandler;
     // Contains (observer-name -> observer-handle) that have ever been registered from
     // previous boots. Observers with all packages expired are periodically pruned.
     // It is saved to disk on system shutdown and repouplated on startup so it survives reboots.
@@ -101,7 +107,7 @@ public class PackageWatchdog {
         mPolicyFile = new AtomicFile(new File(new File(Environment.getDataDirectory(), "system"),
                         "package-watchdog.xml"));
         mTimerHandler = new Handler(Looper.myLooper());
-        mIoHandler = BackgroundThread.getHandler();
+        mWorkerHandler = BackgroundThread.getHandler();
         mPackageCleanup = this::rescheduleCleanup;
         loadFromFile();
     }
@@ -115,7 +121,7 @@ public class PackageWatchdog {
         mContext = context;
         mPolicyFile = new AtomicFile(new File(context.getFilesDir(), "package-watchdog.xml"));
         mTimerHandler = new Handler(looper);
-        mIoHandler = mTimerHandler;
+        mWorkerHandler = mTimerHandler;
         mPackageCleanup = this::rescheduleCleanup;
         loadFromFile();
     }
@@ -228,49 +234,46 @@ public class PackageWatchdog {
     /**
      * Called when a process fails either due to a crash or ANR.
      *
-     * <p>All registered observers for the packages contained in the process will be notified in
-     * order of priority until an observer signifies that it has taken action and other observers
-     * should not notified.
+     * <p>For each package contained in the process, one registered observer with the least user
+     * impact will be notified for mitigation.
      *
      * <p>This method could be called frequently if there is a severe problem on the device.
      */
     public void onPackageFailure(String[] packages) {
-        ArrayMap<String, List<PackageHealthObserver>> packagesToReport = new ArrayMap<>();
-        synchronized (mLock) {
-            if (mAllObservers.isEmpty()) {
-                return;
-            }
+        mWorkerHandler.post(() -> {
+            synchronized (mLock) {
+                if (mAllObservers.isEmpty()) {
+                    return;
+                }
 
-            for (int pIndex = 0; pIndex < packages.length; pIndex++) {
-                // Observers interested in receiving packageName failures
-                List<PackageHealthObserver> observersToNotify = new ArrayList<>();
-                for (int oIndex = 0; oIndex < mAllObservers.size(); oIndex++) {
-                    PackageHealthObserver registeredObserver =
-                            mAllObservers.valueAt(oIndex).mRegisteredObserver;
-                    if (registeredObserver != null) {
-                        observersToNotify.add(registeredObserver);
+                for (int pIndex = 0; pIndex < packages.length; pIndex++) {
+                    String packageToReport = packages[pIndex];
+                    // Observer that will receive failure for packageToReport
+                    PackageHealthObserver currentObserverToNotify = null;
+                    int currentObserverImpact = Integer.MAX_VALUE;
+
+                    // Find observer with least user impact
+                    for (int oIndex = 0; oIndex < mAllObservers.size(); oIndex++) {
+                        ObserverInternal observer = mAllObservers.valueAt(oIndex);
+                        PackageHealthObserver registeredObserver = observer.mRegisteredObserver;
+                        if (registeredObserver != null
+                                && observer.onPackageFailure(packageToReport)) {
+                            int impact = registeredObserver.onHealthCheckFailed(packageToReport);
+                            if (impact != PackageHealthObserverImpact.USER_IMPACT_NONE
+                                    && impact < currentObserverImpact) {
+                                currentObserverToNotify = registeredObserver;
+                                currentObserverImpact = impact;
+                            }
+                        }
                     }
-                }
-                // Save interested observers and notify them outside the lock
-                if (!observersToNotify.isEmpty()) {
-                    packagesToReport.put(packages[pIndex], observersToNotify);
-                }
-            }
-        }
 
-        // Notify observers
-        for (int pIndex = 0; pIndex < packagesToReport.size(); pIndex++) {
-            List<PackageHealthObserver> observers = packagesToReport.valueAt(pIndex);
-            String packageName = packages[pIndex];
-            for (int oIndex = 0; oIndex < observers.size(); oIndex++) {
-                PackageHealthObserver observer = observers.get(oIndex);
-                if (mAllObservers.get(observer.getName()).onPackageFailure(packageName)
-                        && observer.onHealthCheckFailed(packageName)) {
-                    // Observer has handled, do not notify others
-                    break;
+                    // Execute action with least user impact
+                    if (currentObserverToNotify != null) {
+                        currentObserverToNotify.execute(packageToReport);
+                    }
                 }
             }
-        }
+        });
     }
 
     // TODO(zezeozue): Optimize write? Maybe only write a separate smaller file?
@@ -278,21 +281,46 @@ public class PackageWatchdog {
     /** Writes the package information to file during shutdown. */
     public void writeNow() {
         if (!mAllObservers.isEmpty()) {
-            mIoHandler.removeCallbacks(this::saveToFile);
+            mWorkerHandler.removeCallbacks(this::saveToFile);
             pruneObservers(SystemClock.uptimeMillis() - mUptimeAtLastRescheduleMs);
             saveToFile();
             Slog.i(TAG, "Last write to update package durations");
         }
     }
 
+    /** Possible severity values of the user impact of a {@link PackageHealthObserver#execute}. */
+    @Retention(SOURCE)
+    @IntDef(value = {PackageHealthObserverImpact.USER_IMPACT_NONE,
+                     PackageHealthObserverImpact.USER_IMPACT_LOW,
+                     PackageHealthObserverImpact.USER_IMPACT_MEDIUM,
+                     PackageHealthObserverImpact.USER_IMPACT_HIGH})
+    public @interface PackageHealthObserverImpact {
+        /** No action to take. */
+        int USER_IMPACT_NONE = 0;
+        /* Action has low user impact, user of a device will barely notice. */
+        int USER_IMPACT_LOW = 1;
+        /* Action has medium user impact, user of a device will likely notice. */
+        int USER_IMPACT_MEDIUM = 3;
+        /* Action has high user impact, a last resort, user of a device will be very frustrated. */
+        int USER_IMPACT_HIGH = 5;
+    }
+
     /** Register instances of this interface to receive notifications on package failure. */
     public interface PackageHealthObserver {
         /**
          * Called when health check fails for the {@code packageName}.
-         * @return {@code true} if action was taken and other observers should not be notified of
-         * this failure, {@code false} otherwise.
+         *
+         * @return any one of {@link PackageHealthObserverImpact} to express the impact
+         * to the user on {@link #execute}
          */
-        boolean onHealthCheckFailed(String packageName);
+        @PackageHealthObserverImpact int onHealthCheckFailed(String packageName);
+
+        /**
+         * Executes mitigation for {@link #onHealthCheckFailed}.
+         *
+         * @return {@code true} if action was executed successfully, {@code false} otherwise
+         */
+        boolean execute(String packageName);
 
         // TODO(zezeozue): Ensure uniqueness?
         /**
@@ -442,8 +470,8 @@ public class PackageWatchdog {
     }
 
     private void saveToFileAsync() {
-        mIoHandler.removeCallbacks(this::saveToFile);
-        mIoHandler.post(this::saveToFile);
+        mWorkerHandler.removeCallbacks(this::saveToFile);
+        mWorkerHandler.post(this::saveToFile);
     }
 
     /**
@@ -606,7 +634,11 @@ public class PackageWatchdog {
             } else {
                 mFailures++;
             }
-            return mFailures >= TRIGGER_FAILURE_COUNT;
+            boolean failed = mFailures >= TRIGGER_FAILURE_COUNT;
+            if (failed) {
+                mFailures = 0;
+            }
+            return failed;
         }
     }
 }
diff --git a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java
index 3954a1178e09..a4ef8dc3049b 100644
--- a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java
+++ b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java
@@ -27,6 +27,7 @@ import android.os.HandlerThread;
 
 import com.android.server.PackageWatchdog;
 import com.android.server.PackageWatchdog.PackageHealthObserver;
+import com.android.server.PackageWatchdog.PackageHealthObserverImpact;
 
 import java.util.List;
 
@@ -39,10 +40,12 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve
     private static final String TAG = "RollbackPackageHealthObserver";
     private static final String NAME = "rollback-observer";
     private Context mContext;
+    private RollbackManager mRollbackManager;
     private Handler mHandler;
 
     RollbackPackageHealthObserver(Context context) {
         mContext = context;
+        mRollbackManager = mContext.getSystemService(RollbackManager.class);
         HandlerThread handlerThread = new HandlerThread("RollbackPackageHealthObserver");
         handlerThread.start();
         mHandler = handlerThread.getThreadHandler();
@@ -50,19 +53,47 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve
     }
 
     @Override
-    public boolean onHealthCheckFailed(String packageName) {
-        RollbackManager rollbackManager = mContext.getSystemService(RollbackManager.class);
-        for (RollbackInfo rollback : rollbackManager.getAvailableRollbacks()) {
-            for (PackageRollbackInfo packageRollback : rollback.getPackages()) {
-                if (packageName.equals(packageRollback.getPackageName())) {
-                    // TODO(zezeozue): Only rollback if rollback version == failed package version
-                    mHandler.post(() -> executeRollback(rollbackManager, rollback));
-                    return true;
-                }
-            }
+    public int onHealthCheckFailed(String packageName) {
+        RollbackInfo rollback = getAvailableRollback(packageName);
+        if (rollback == null) {
+            // Don't handle the notification, no rollbacks available for the package
+            return PackageHealthObserverImpact.USER_IMPACT_NONE;
         }
-        // Don't handle the notification, no rollbacks available
-        return false;
+        // Rollback is available, we may get a callback into #execute
+        return PackageHealthObserverImpact.USER_IMPACT_MEDIUM;
+    }
+
+    @Override
+    public boolean execute(String packageName) {
+        RollbackInfo rollback = getAvailableRollback(packageName);
+        if (rollback == null) {
+            // Expected a rollback to be available, what happened?
+            return false;
+        }
+
+        // TODO(zezeozue): Only rollback if rollback version == failed package version
+        LocalIntentReceiver rollbackReceiver = new LocalIntentReceiver((Intent result) -> {
+            int status = result.getIntExtra(PackageInstaller.EXTRA_STATUS,
+                    PackageInstaller.STATUS_FAILURE);
+            if (status == PackageInstaller.STATUS_SUCCESS) {
+                // TODO(zezeozue); Log success metrics
+                // Rolledback successfully, no action required by other observers
+            } else {
+                // TODO(zezeozue); Log failure metrics
+                // Rollback failed other observers should have a shot
+            }
+        });
+
+        // TODO(zezeozue): Log initiated metrics
+        mHandler.post(() ->
+                mRollbackManager.commitRollback(rollback, rollbackReceiver.getIntentSender()));
+        // Assume rollback executed successfully
+        return true;
+    }
+
+    @Override
+    public String getName() {
+        return NAME;
     }
 
     /**
@@ -73,26 +104,15 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve
         PackageWatchdog.getInstance(mContext).startObservingHealth(this, packages, durationMs);
     }
 
-    private void executeRollback(RollbackManager manager, RollbackInfo rollback) {
-        // TODO(zezeozue): Log initiated metrics
-        LocalIntentReceiver rollbackReceiver = new LocalIntentReceiver((Intent result) -> {
-            mHandler.post(() -> {
-                int status = result.getIntExtra(PackageInstaller.EXTRA_STATUS,
-                        PackageInstaller.STATUS_FAILURE);
-                if (status == PackageInstaller.STATUS_SUCCESS) {
-                    // TODO(zezeozue); Log success metrics
-                    // Rolledback successfully, no action required by other observers
-                } else {
-                    // TODO(zezeozue); Log failure metrics
-                    // Rollback failed other observers should have a shot
+    private RollbackInfo getAvailableRollback(String packageName) {
+        for (RollbackInfo rollback : mRollbackManager.getAvailableRollbacks()) {
+            for (PackageRollbackInfo packageRollback : rollback.getPackages()) {
+                if (packageName.equals(packageRollback.getPackageName())) {
+                    // TODO(zezeozue): Only rollback if rollback version == failed package version
+                    return rollback;
                 }
-            });
-        });
-        manager.commitRollback(rollback, rollbackReceiver.getIntentSender());
-    }
-
-    @Override
-    public String getName() {
-        return NAME;
+            }
+        }
+        return null;
     }
 }
diff --git a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
index ec07037b3a8f..86af6422dad3 100644
--- a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
+++ b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
@@ -26,8 +26,8 @@ import android.os.test.TestLooper;
 import android.support.test.InstrumentationRegistry;
 
 import com.android.server.PackageWatchdog.PackageHealthObserver;
+import com.android.server.PackageWatchdog.PackageHealthObserverImpact;
 
-import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -45,23 +45,22 @@ import java.util.concurrent.TimeUnit;
 public class PackageWatchdogTest {
     private static final String APP_A = "com.package.a";
     private static final String APP_B = "com.package.b";
+    private static final String APP_C = "com.package.c";
+    private static final String APP_D = "com.package.d";
     private static final String OBSERVER_NAME_1 = "observer1";
     private static final String OBSERVER_NAME_2 = "observer2";
     private static final String OBSERVER_NAME_3 = "observer3";
+    private static final String OBSERVER_NAME_4 = "observer4";
     private static final long SHORT_DURATION = TimeUnit.SECONDS.toMillis(1);
     private static final long LONG_DURATION = TimeUnit.SECONDS.toMillis(5);
     private TestLooper mTestLooper;
 
     @Before
     public void setUp() throws Exception {
-        mTestLooper = new TestLooper();
-        mTestLooper.startAutoDispatch();
-    }
-
-    @After
-    public void tearDown() throws Exception {
         new File(InstrumentationRegistry.getContext().getFilesDir(),
                 "package-watchdog.xml").delete();
+        mTestLooper = new TestLooper();
+        mTestLooper.startAutoDispatch();
     }
 
     /**
@@ -154,7 +153,6 @@ public class PackageWatchdogTest {
         assertTrue(watchdog1.getPackages(observer2).contains(APP_A));
         assertTrue(watchdog1.getPackages(observer2).contains(APP_B));
 
-
         // Then advance time and run IO Handler so file is saved
         mTestLooper.dispatchAll();
 
@@ -198,47 +196,191 @@ public class PackageWatchdogTest {
             watchdog.onPackageFailure(new String[]{APP_A});
         }
 
+        // Run handler so package failures are dispatched to observers
+        mTestLooper.dispatchAll();
+
         // Verify that observers are not notified
         assertEquals(0, observer1.mFailedPackages.size());
         assertEquals(0, observer2.mFailedPackages.size());
     }
 
     /**
-     * Test package failure and notifies all observer since none handles the failure
+     * Test package failure and does not notify any observer because they are not observing
+     * the failed packages.
      */
     @Test
-    public void testPackageFailureNotifyAll() throws Exception {
+    public void testPackageFailureNotifyNone() throws Exception {
         PackageWatchdog watchdog = createWatchdog();
-        TestObserver observer1 = new TestObserver(OBSERVER_NAME_1);
-        TestObserver observer2 = new TestObserver(OBSERVER_NAME_2);
+        TestObserver observer1 = new TestObserver(OBSERVER_NAME_1,
+                PackageHealthObserverImpact.USER_IMPACT_HIGH);
+        TestObserver observer2 = new TestObserver(OBSERVER_NAME_2,
+                PackageHealthObserverImpact.USER_IMPACT_HIGH);
+
 
-        // Start observing for observer1 and observer2 without handling failures
         watchdog.startObservingHealth(observer2, Arrays.asList(APP_A), SHORT_DURATION);
-        watchdog.startObservingHealth(observer1, Arrays.asList(APP_A, APP_B), SHORT_DURATION);
+        watchdog.startObservingHealth(observer1, Arrays.asList(APP_B), SHORT_DURATION);
+
+        // Then fail APP_C (not observed) above the threshold
+        for (int i = 0; i < TRIGGER_FAILURE_COUNT; i++) {
+            watchdog.onPackageFailure(new String[]{APP_C});
+        }
+
+        // Run handler so package failures are dispatched to observers
+        mTestLooper.dispatchAll();
 
-        // Then fail APP_A and APP_B above the threshold
+        // Verify that observers are not notified
+        assertEquals(0, observer1.mFailedPackages.size());
+        assertEquals(0, observer2.mFailedPackages.size());
+    }
+
+    /**
+     * Test package failure and notifies only least impact observers.
+     */
+    @Test
+    public void testPackageFailureNotifyAllDifferentImpacts() throws Exception {
+        PackageWatchdog watchdog = createWatchdog();
+        TestObserver observerNone = new TestObserver(OBSERVER_NAME_1,
+                PackageHealthObserverImpact.USER_IMPACT_NONE);
+        TestObserver observerHigh = new TestObserver(OBSERVER_NAME_2,
+                PackageHealthObserverImpact.USER_IMPACT_HIGH);
+        TestObserver observerMid = new TestObserver(OBSERVER_NAME_3,
+                PackageHealthObserverImpact.USER_IMPACT_MEDIUM);
+        TestObserver observerLow = new TestObserver(OBSERVER_NAME_4,
+                PackageHealthObserverImpact.USER_IMPACT_LOW);
+
+        // Start observing for all impact observers
+        watchdog.startObservingHealth(observerNone, Arrays.asList(APP_A, APP_B, APP_C, APP_D),
+                SHORT_DURATION);
+        watchdog.startObservingHealth(observerHigh, Arrays.asList(APP_A, APP_B, APP_C),
+                SHORT_DURATION);
+        watchdog.startObservingHealth(observerMid, Arrays.asList(APP_A, APP_B),
+                SHORT_DURATION);
+        watchdog.startObservingHealth(observerLow, Arrays.asList(APP_A),
+                SHORT_DURATION);
+
+        // Then fail all apps above the threshold
         for (int i = 0; i < TRIGGER_FAILURE_COUNT; i++) {
-            watchdog.onPackageFailure(new String[]{APP_A, APP_B});
+            watchdog.onPackageFailure(new String[]{APP_A, APP_B, APP_C, APP_D});
         }
 
-        // Verify all observers are notifed of all package failures
-        List<String> observer1Packages = observer1.mFailedPackages;
-        List<String> observer2Packages = observer2.mFailedPackages;
-        assertEquals(2, observer1Packages.size());
-        assertEquals(1, observer2Packages.size());
-        assertEquals(APP_A, observer1Packages.get(0));
-        assertEquals(APP_B, observer1Packages.get(1));
-        assertEquals(APP_A, observer2Packages.get(0));
+        // Run handler so package failures are dispatched to observers
+        mTestLooper.dispatchAll();
+
+        // Verify least impact observers are notifed of package failures
+        List<String> observerNonePackages = observerNone.mFailedPackages;
+        List<String> observerHighPackages = observerHigh.mFailedPackages;
+        List<String> observerMidPackages = observerMid.mFailedPackages;
+        List<String> observerLowPackages = observerLow.mFailedPackages;
+
+        // APP_D failure observed by only observerNone is not caught cos its impact is none
+        assertEquals(0, observerNonePackages.size());
+        // APP_C failure is caught by observerHigh cos it's the lowest impact observer
+        assertEquals(1, observerHighPackages.size());
+        assertEquals(APP_C, observerHighPackages.get(0));
+        // APP_B failure is caught by observerMid cos it's the lowest impact observer
+        assertEquals(1, observerMidPackages.size());
+        assertEquals(APP_B, observerMidPackages.get(0));
+        // APP_A failure is caught by observerLow cos it's the lowest impact observer
+        assertEquals(1, observerLowPackages.size());
+        assertEquals(APP_A, observerLowPackages.get(0));
     }
 
     /**
-     * Test package failure and notifies only one observer because it handles the failure
+     * Test package failure and least impact observers are notified successively.
+     * State transistions:
+     *
+     * <ul>
+     * <li>(observer1:low, observer2:mid) -> {observer1}
+     * <li>(observer1:high, observer2:mid) -> {observer2}
+     * <li>(observer1:high, observer2:none) -> {observer1}
+     * <li>(observer1:none, observer2:none) -> {}
+     * <ul>
      */
     @Test
-    public void testPackageFailureNotifyOne() throws Exception {
+    public void testPackageFailureNotifyLeastSuccessively() throws Exception {
         PackageWatchdog watchdog = createWatchdog();
-        TestObserver observer1 = new TestObserver(OBSERVER_NAME_1, true /* shouldHandle */);
-        TestObserver observer2 = new TestObserver(OBSERVER_NAME_2, true /* shouldHandle */);
+        TestObserver observerFirst = new TestObserver(OBSERVER_NAME_1,
+                PackageHealthObserverImpact.USER_IMPACT_LOW);
+        TestObserver observerSecond = new TestObserver(OBSERVER_NAME_2,
+                PackageHealthObserverImpact.USER_IMPACT_MEDIUM);
+
+        // Start observing for observerFirst and observerSecond with failure handling
+        watchdog.startObservingHealth(observerFirst, Arrays.asList(APP_A), LONG_DURATION);
+        watchdog.startObservingHealth(observerSecond, Arrays.asList(APP_A), LONG_DURATION);
+
+        // Then fail APP_A above the threshold
+        for (int i = 0; i < TRIGGER_FAILURE_COUNT; i++) {
+            watchdog.onPackageFailure(new String[]{APP_A});
+        }
+        // Run handler so package failures are dispatched to observers
+        mTestLooper.dispatchAll();
+
+        // Verify only observerFirst is notifed
+        assertEquals(1, observerFirst.mFailedPackages.size());
+        assertEquals(APP_A, observerFirst.mFailedPackages.get(0));
+        assertEquals(0, observerSecond.mFailedPackages.size());
+
+        // After observerFirst handles failure, next action it has is high impact
+        observerFirst.mImpact = PackageHealthObserverImpact.USER_IMPACT_HIGH;
+        observerFirst.mFailedPackages.clear();
+        observerSecond.mFailedPackages.clear();
+
+        // Then fail APP_A again above the threshold
+        for (int i = 0; i < TRIGGER_FAILURE_COUNT; i++) {
+            watchdog.onPackageFailure(new String[]{APP_A});
+        }
+        // Run handler so package failures are dispatched to observers
+        mTestLooper.dispatchAll();
+
+        // Verify only observerSecond is notifed cos it has least impact
+        assertEquals(1, observerSecond.mFailedPackages.size());
+        assertEquals(APP_A, observerSecond.mFailedPackages.get(0));
+        assertEquals(0, observerFirst.mFailedPackages.size());
+
+        // After observerSecond handles failure, it has no further actions
+        observerSecond.mImpact = PackageHealthObserverImpact.USER_IMPACT_NONE;
+        observerFirst.mFailedPackages.clear();
+        observerSecond.mFailedPackages.clear();
+
+        // Then fail APP_A again above the threshold
+        for (int i = 0; i < TRIGGER_FAILURE_COUNT; i++) {
+            watchdog.onPackageFailure(new String[]{APP_A});
+        }
+        // Run handler so package failures are dispatched to observers
+        mTestLooper.dispatchAll();
+
+        // Verify only observerFirst is notifed cos it has the only action
+        assertEquals(1, observerFirst.mFailedPackages.size());
+        assertEquals(APP_A, observerFirst.mFailedPackages.get(0));
+        assertEquals(0, observerSecond.mFailedPackages.size());
+
+        // After observerFirst handles failure, it too has no further actions
+        observerFirst.mImpact = PackageHealthObserverImpact.USER_IMPACT_NONE;
+        observerFirst.mFailedPackages.clear();
+        observerSecond.mFailedPackages.clear();
+
+        // Then fail APP_A again above the threshold
+        for (int i = 0; i < TRIGGER_FAILURE_COUNT; i++) {
+            watchdog.onPackageFailure(new String[]{APP_A});
+        }
+        // Run handler so package failures are dispatched to observers
+        mTestLooper.dispatchAll();
+
+        // Verify no observer is notified cos no actions left
+        assertEquals(0, observerFirst.mFailedPackages.size());
+        assertEquals(0, observerSecond.mFailedPackages.size());
+    }
+
+    /**
+     * Test package failure and notifies only one observer even with observer impact tie.
+     */
+    @Test
+    public void testPackageFailureNotifyOneSameImpact() throws Exception {
+        PackageWatchdog watchdog = createWatchdog();
+        TestObserver observer1 = new TestObserver(OBSERVER_NAME_1,
+                PackageHealthObserverImpact.USER_IMPACT_HIGH);
+        TestObserver observer2 = new TestObserver(OBSERVER_NAME_2,
+                PackageHealthObserverImpact.USER_IMPACT_HIGH);
 
         // Start observing for observer1 and observer2 with failure handling
         watchdog.startObservingHealth(observer2, Arrays.asList(APP_A), SHORT_DURATION);
@@ -249,6 +391,9 @@ public class PackageWatchdogTest {
             watchdog.onPackageFailure(new String[]{APP_A});
         }
 
+        // Run handler so package failures are dispatched to observers
+        mTestLooper.dispatchAll();
+
         // Verify only one observer is notifed
         assertEquals(1, observer1.mFailedPackages.size());
         assertEquals(APP_A, observer1.mFailedPackages.get(0));
@@ -262,21 +407,26 @@ public class PackageWatchdogTest {
 
     private static class TestObserver implements PackageHealthObserver {
         private final String mName;
-        private boolean mShouldHandle;
+        private int mImpact;
         final List<String> mFailedPackages = new ArrayList<>();
 
         TestObserver(String name) {
             mName = name;
+            mImpact = PackageHealthObserverImpact.USER_IMPACT_MEDIUM;
         }
 
-        TestObserver(String name, boolean shouldHandle) {
+        TestObserver(String name, int impact) {
             mName = name;
-            mShouldHandle = shouldHandle;
+            mImpact = impact;
+        }
+
+        public int onHealthCheckFailed(String packageName) {
+            return mImpact;
         }
 
-        public boolean onHealthCheckFailed(String packageName) {
+        public boolean execute(String packageName) {
             mFailedPackages.add(packageName);
-            return mShouldHandle;
+            return true;
         }
 
         public String getName() {