Merge changes I7dfea957,I2b3ce79e,Iefd31d08,I44c51d0d,Iacad3a2f into sc-dev

* changes:
  Register and unregister might be out of order (5/n)
  Fix a race in setExplicitHealthCheckEnabled() (4/n)
  updateConfigs() might throw during tests (3/n)
  Set device configs before starting tests (2/n)
  Exhaust due runnables before time-leap (1/n)

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

@@ -372,10 +372,12 @@ public class PackageWatchdog {
      * even from a previous boot.
      */
     public void unregisterHealthObserver(PackageHealthObserver observer) {
+        mLongTaskHandler.post(() -> {
             synchronized (mLock) {
                 mAllObservers.remove(observer.getName());
             }
             syncState("unregistering observer: " + observer.getName());
+        });
     }
 
     /**
@@ -982,7 +984,11 @@ public class PackageWatchdog {
                     if (!DeviceConfig.NAMESPACE_ROLLBACK.equals(properties.getNamespace())) {
                         return;
                     }
+                    try {
                         updateConfigs();
+                    } catch (Exception ignore) {
+                        Slog.w(TAG, "Failed to reload device config changes");
+                    }
                 });
     }
 
@@ -990,7 +996,8 @@ public class PackageWatchdog {
      * Health check is enabled or disabled after reading the flags
      * from DeviceConfig.
      */
-    private void updateConfigs() {
+    @VisibleForTesting
+    void updateConfigs() {
         synchronized (mLock) {
             mTriggerFailureCount = DeviceConfig.getInt(
                     DeviceConfig.NAMESPACE_ROLLBACK,

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

@@ -98,6 +98,8 @@ public class PackageWatchdogTest {
     private final TestClock mTestClock = new TestClock();
     private TestLooper mTestLooper;
     private Context mSpyContext;
+    // Keep track of all created watchdogs to apply device config changes
+    private List<PackageWatchdog> mAllocatedWatchdogs;
     @Mock
     private ConnectivityModuleConnector mConnectivityModuleConnector;
     @Mock
@@ -112,7 +114,8 @@ public class PackageWatchdogTest {
         MockitoAnnotations.initMocks(this);
         new File(InstrumentationRegistry.getContext().getFilesDir(),
                 "package-watchdog.xml").delete();
-        adoptShellPermissions(Manifest.permission.READ_DEVICE_CONFIG);
+        adoptShellPermissions(Manifest.permission.READ_DEVICE_CONFIG,
+                Manifest.permission.WRITE_DEVICE_CONFIG);
         mTestLooper = new TestLooper();
         mSpyContext = spy(InstrumentationRegistry.getContext());
         when(mSpyContext.getPackageManager()).thenReturn(mMockPackageManager);
@@ -157,12 +160,23 @@ public class PackageWatchdogTest {
                     return storedValue == null ? defaultValue : Long.parseLong(storedValue);
                 }
         ).when(() -> SystemProperties.getLong(anyString(), anyLong()));
+
+        DeviceConfig.setProperty(DeviceConfig.NAMESPACE_ROLLBACK,
+                PackageWatchdog.PROPERTY_WATCHDOG_EXPLICIT_HEALTH_CHECK_ENABLED,
+                Boolean.toString(true), false);
+
+        DeviceConfig.setProperty(DeviceConfig.NAMESPACE_ROLLBACK,
+                PackageWatchdog.PROPERTY_WATCHDOG_TRIGGER_FAILURE_COUNT,
+                Integer.toString(PackageWatchdog.DEFAULT_TRIGGER_FAILURE_COUNT), false);
+
+        mAllocatedWatchdogs = new ArrayList<>();
     }
 
     @After
     public void tearDown() throws Exception {
         dropShellPermissions();
         mSession.finishMocking();
+        mAllocatedWatchdogs.clear();
     }
 
     @Test
@@ -611,10 +625,6 @@ public class PackageWatchdogTest {
      */
     @Test
     public void testExplicitHealthCheckStateChanges() throws Exception {
-        adoptShellPermissions(
-                Manifest.permission.WRITE_DEVICE_CONFIG,
-                Manifest.permission.READ_DEVICE_CONFIG);
-
         TestController controller = new TestController();
         PackageWatchdog watchdog = createWatchdog(controller, true /* withPackagesReady */);
         TestObserver observer = new TestObserver(OBSERVER_NAME_1,
@@ -807,9 +817,6 @@ public class PackageWatchdogTest {
     /** Test default values are used when device property is invalid. */
     @Test
     public void testInvalidConfig_watchdogTriggerFailureCount() {
-        adoptShellPermissions(
-                Manifest.permission.WRITE_DEVICE_CONFIG,
-                Manifest.permission.READ_DEVICE_CONFIG);
         DeviceConfig.setProperty(DeviceConfig.NAMESPACE_ROLLBACK,
                 PackageWatchdog.PROPERTY_WATCHDOG_TRIGGER_FAILURE_COUNT,
                 Integer.toString(-1), /*makeDefault*/false);
@@ -835,9 +842,6 @@ public class PackageWatchdogTest {
     /** Test default values are used when device property is invalid. */
     @Test
     public void testInvalidConfig_watchdogTriggerDurationMillis() {
-        adoptShellPermissions(
-                Manifest.permission.WRITE_DEVICE_CONFIG,
-                Manifest.permission.READ_DEVICE_CONFIG);
         DeviceConfig.setProperty(DeviceConfig.NAMESPACE_ROLLBACK,
                 PackageWatchdog.PROPERTY_WATCHDOG_TRIGGER_FAILURE_COUNT,
                 Integer.toString(2), /*makeDefault*/false);
@@ -850,7 +854,6 @@ public class PackageWatchdogTest {
         watchdog.startObservingHealth(observer, Arrays.asList(APP_A, APP_B), Long.MAX_VALUE);
         watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE)),
                 PackageWatchdog.FAILURE_REASON_UNKNOWN);
-        mTestLooper.dispatchAll();
         moveTimeForwardAndDispatch(PackageWatchdog.DEFAULT_TRIGGER_FAILURE_DURATION_MS + 1);
         watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE)),
                 PackageWatchdog.FAILURE_REASON_UNKNOWN);
@@ -862,7 +865,6 @@ public class PackageWatchdogTest {
 
         watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_B, VERSION_CODE)),
                 PackageWatchdog.FAILURE_REASON_UNKNOWN);
-        mTestLooper.dispatchAll();
         moveTimeForwardAndDispatch(PackageWatchdog.DEFAULT_TRIGGER_FAILURE_DURATION_MS - 1);
         watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_B, VERSION_CODE)),
                 PackageWatchdog.FAILURE_REASON_UNKNOWN);
@@ -917,9 +919,6 @@ public class PackageWatchdogTest {
     /** Test we are notified when enough failures are triggered within any window. */
     @Test
     public void testFailureTriggerWindow() {
-        adoptShellPermissions(
-                Manifest.permission.WRITE_DEVICE_CONFIG,
-                Manifest.permission.READ_DEVICE_CONFIG);
         DeviceConfig.setProperty(DeviceConfig.NAMESPACE_ROLLBACK,
                 PackageWatchdog.PROPERTY_WATCHDOG_TRIGGER_FAILURE_COUNT,
                 Integer.toString(3), /*makeDefault*/false);
@@ -933,11 +932,9 @@ public class PackageWatchdogTest {
         // Raise 2 failures at t=0 and t=900 respectively
         watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE)),
                 PackageWatchdog.FAILURE_REASON_UNKNOWN);
-        mTestLooper.dispatchAll();
         moveTimeForwardAndDispatch(900);
         watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE)),
                 PackageWatchdog.FAILURE_REASON_UNKNOWN);
-        mTestLooper.dispatchAll();
 
         // Raise 2 failures at t=1100
         moveTimeForwardAndDispatch(200);
@@ -1303,15 +1300,15 @@ public class PackageWatchdogTest {
         DeviceConfig.setProperty(DeviceConfig.NAMESPACE_ROLLBACK,
                 PackageWatchdog.PROPERTY_WATCHDOG_EXPLICIT_HEALTH_CHECK_ENABLED,
                 Boolean.toString(enabled), /*makeDefault*/false);
-        //give time for DeviceConfig to broadcast the property value change
+        // Call updateConfigs() so device config changes take effect immediately
-        try {
+        for (PackageWatchdog watchdog : mAllocatedWatchdogs) {
-            Thread.sleep(SHORT_DURATION);
+            watchdog.updateConfigs();
-        } catch (InterruptedException e) {
-            fail("Thread.sleep unexpectedly failed!");
         }
     }
 
     private void moveTimeForwardAndDispatch(long milliSeconds) {
+        // Exhaust all due runnables now which shouldn't be executed after time-leap
+        mTestLooper.dispatchAll();
         mTestClock.moveTimeForward(milliSeconds);
         mTestLooper.moveTimeForward(milliSeconds);
         mTestLooper.dispatchAll();
@@ -1354,6 +1351,7 @@ public class PackageWatchdogTest {
             verify(mConnectivityModuleConnector).registerHealthListener(
                     mConnectivityModuleCallbackCaptor.capture());
         }
+        mAllocatedWatchdogs.add(watchdog);
         return watchdog;
     }