From 56b46738d79cc7725358edab03ae3fb02e74eef4 Mon Sep 17 00:00:00 2001 From: Gavin Corkery Date: Thu, 12 Mar 2020 13:17:09 +0000 Subject: [PATCH] Revert "Only sync requests when there are changes to report" This reverts commit 68713d98304f942d365e12e4cd63253a13eff32f. Reason for revert: This breaks NetworkStagedRollbackTest. I know the cause, but reverting for now is the best option Bug: 151212665 Change-Id: I00f0d3011b7ad2b65d452b3d74e508ffc4d1a857 --- .../com/android/server/PackageWatchdog.java | 18 ++----- .../android/server/PackageWatchdogTest.java | 52 ------------------- 2 files changed, 5 insertions(+), 65 deletions(-) diff --git a/services/core/java/com/android/server/PackageWatchdog.java b/services/core/java/com/android/server/PackageWatchdog.java index 2cfe404f8c6f7..b464422e9e3da 100644 --- a/services/core/java/com/android/server/PackageWatchdog.java +++ b/services/core/java/com/android/server/PackageWatchdog.java @@ -161,9 +161,6 @@ 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 mRequestedHealthCheckPackages = new ArraySet<>(); @GuardedBy("mLock") private boolean mIsPackagesReady; // Flag to control whether explicit health checks are supported or not @@ -627,22 +624,17 @@ public class PackageWatchdog { * @see #syncRequestsAsync */ private void syncRequests() { - boolean syncRequired = false; + Set packages = null; synchronized (mLock) { if (mIsPackagesReady) { - Set packages = getPackagesPendingHealthChecksLocked(); - if (!packages.equals(mRequestedHealthCheckPackages)) { - syncRequired = true; - mRequestedHealthCheckPackages = packages; - } + packages = getPackagesPendingHealthChecksLocked(); } // else, we will sync requests when packages become ready } // Call outside lock to avoid holding lock when calling into the controller. - if (syncRequired) { - Slog.i(TAG, "Syncing health check requests for packages: " - + mRequestedHealthCheckPackages); - mHealthCheckController.syncRequests(mRequestedHealthCheckPackages); + if (packages != null) { + Slog.i(TAG, "Syncing health check requests for packages: " + packages); + mHealthCheckController.syncRequests(packages); } } diff --git a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java index 819fc020e3971..8cc8cf4d2a970 100644 --- a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java +++ b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java @@ -1063,52 +1063,6 @@ 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 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() @@ -1265,7 +1219,6 @@ public class PackageWatchdogTest { private Consumer mPassedConsumer; private Consumer> mSupportedConsumer; private Runnable mNotifySyncRunnable; - private List mSyncRequests = new ArrayList<>(); @Override public void setEnabled(boolean enabled) { @@ -1285,7 +1238,6 @@ public class PackageWatchdogTest { @Override public void syncRequests(Set packages) { - mSyncRequests.add(packages); mRequestedPackages.clear(); if (mIsEnabled) { packages.retainAll(mSupportedPackages); @@ -1316,10 +1268,6 @@ public class PackageWatchdogTest { return Collections.emptyList(); } } - - public List getSyncRequests() { - return mSyncRequests; - } } private static class TestClock implements PackageWatchdog.SystemClock {