Support for Watchable verification
Bug: 161323622
Add support to automate verification of Watched attributes. It is now
possible to verify that all Watched attributes of class Watchable are
being observed. It is also possible to know that Watched attributes
of class Snappable have been copied properly into a snapshot.
Tested on Settings by commenting-out a registerObserver() call and
verifying that an exception was thrown.
Test: atest
* FrameworksServicesTests:WatcherTest
* FrameworksServicesTests:PackageManagerSettingsTests
* FrameworksServicesTests:AppsFilterTest
Change-Id: Ia2bff16bec7bff0b8b8e401ad5377bad8297b527
diff --git a/services/core/java/com/android/server/WatchableIntentResolver.java b/services/core/java/com/android/server/WatchableIntentResolver.java
index 3b5d168..2ef94f1 100644
--- a/services/core/java/com/android/server/WatchableIntentResolver.java
+++ b/services/core/java/com/android/server/WatchableIntentResolver.java
@@ -39,28 +39,45 @@
* Watchable machinery
*/
private final Watchable mWatchable = new WatchableImpl();
+
/**
* Register an observer to receive change notifications.
* @param observer The observer to register.
*/
+ @Override
public void registerObserver(@NonNull Watcher observer) {
mWatchable.registerObserver(observer);
}
+
/**
* Unregister the observer, which will no longer receive change notifications.
* @param observer The observer to unregister.
*/
+ @Override
public void unregisterObserver(@NonNull Watcher observer) {
mWatchable.unregisterObserver(observer);
}
+
+ /**
+ * Return true if the {@link Watcher) is a registered observer.
+ * @param observer A {@link Watcher} that might be registered
+ * @return true if the observer is registered with this {@link Watchable}.
+ */
+ @Override
+ public boolean isRegisteredObserver(@NonNull Watcher observer) {
+ return mWatchable.isRegisteredObserver(observer);
+ }
+
/**
* Notify listeners that the object has changd. The argument is a hint as to the
* source of the change.
* @param what The attribute or sub-object that changed, if not null.
*/
+ @Override
public void dispatchChange(@Nullable Watchable what) {
mWatchable.dispatchChange(what);
}
+
/**
* Notify listeners that this object has changed.
*/
diff --git a/services/core/java/com/android/server/pm/AppsFilter.java b/services/core/java/com/android/server/pm/AppsFilter.java
index b76fff5..f8990c0 100644
--- a/services/core/java/com/android/server/pm/AppsFilter.java
+++ b/services/core/java/com/android/server/pm/AppsFilter.java
@@ -167,6 +167,7 @@
*
* @param observer The {@link Watcher} to be notified when the {@link Watchable} changes.
*/
+ @Override
public void registerObserver(@NonNull Watcher observer) {
mWatchable.registerObserver(observer);
}
@@ -177,17 +178,29 @@
*
* @param observer The {@link Watcher} that should not be in the notification list.
*/
+ @Override
public void unregisterObserver(@NonNull Watcher observer) {
mWatchable.unregisterObserver(observer);
}
/**
+ * Return true if the {@link Watcher) is a registered observer.
+ * @param observer A {@link Watcher} that might be registered
+ * @return true if the observer is registered with this {@link Watchable}.
+ */
+ @Override
+ public boolean isRegisteredObserver(@NonNull Watcher observer) {
+ return mWatchable.isRegisteredObserver(observer);
+ }
+
+ /**
* Invokes {@link Watcher#onChange} on each registered observer. The method can be called
* with the {@link Watchable} that generated the event. In a tree of {@link Watchable}s, this
* is generally the first (deepest) {@link Watchable} to detect a change.
*
* @param what The {@link Watchable} that generated the event.
*/
+ @Override
public void dispatchChange(@Nullable Watchable what) {
mSnapshot = null;
mWatchable.dispatchChange(what);
diff --git a/services/core/java/com/android/server/pm/InstantAppRegistry.java b/services/core/java/com/android/server/pm/InstantAppRegistry.java
index 69d3e5c..c3bca28 100644
--- a/services/core/java/com/android/server/pm/InstantAppRegistry.java
+++ b/services/core/java/com/android/server/pm/InstantAppRegistry.java
@@ -154,6 +154,9 @@
public void unregisterObserver(@NonNull Watcher observer) {
mWatchable.unregisterObserver(observer);
}
+ public boolean isRegisteredObserver(@NonNull Watcher observer) {
+ return mWatchable.isRegisteredObserver(observer);
+ }
public void dispatchChange(@Nullable Watchable what) {
mSnapshot = null;
mWatchable.dispatchChange(what);
diff --git a/services/core/java/com/android/server/pm/SettingBase.java b/services/core/java/com/android/server/pm/SettingBase.java
index 7924d5d..0e8a278 100644
--- a/services/core/java/com/android/server/pm/SettingBase.java
+++ b/services/core/java/com/android/server/pm/SettingBase.java
@@ -49,6 +49,7 @@
*
* @param observer The {@link Watcher} to be notified when the {@link Watchable} changes.
*/
+ @Override
public void registerObserver(@NonNull Watcher observer) {
mWatchable.registerObserver(observer);
}
@@ -59,20 +60,33 @@
*
* @param observer The {@link Watcher} that should not be in the notification list.
*/
+ @Override
public void unregisterObserver(@NonNull Watcher observer) {
mWatchable.unregisterObserver(observer);
}
/**
+ * Return true if the {@link Watcher) is a registered observer.
+ * @param observer A {@link Watcher} that might be registered
+ * @return true if the observer is registered with this {@link Watchable}.
+ */
+ @Override
+ public boolean isRegisteredObserver(@NonNull Watcher observer) {
+ return mWatchable.isRegisteredObserver(observer);
+ }
+
+ /**
* Invokes {@link Watcher#onChange} on each registered observer. The method can be called
* with the {@link Watchable} that generated the event. In a tree of {@link Watchable}s, this
* is generally the first (deepest) {@link Watchable} to detect a change.
*
* @param what The {@link Watchable} that generated the event.
*/
+ @Override
public void dispatchChange(@Nullable Watchable what) {
mWatchable.dispatchChange(what);
}
+
/**
* Notify listeners that this object has changed.
*/
diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java
index 2d41a687..50c1065 100644
--- a/services/core/java/com/android/server/pm/Settings.java
+++ b/services/core/java/com/android/server/pm/Settings.java
@@ -192,6 +192,16 @@
}
/**
+ * Return true if the {@link Watcher) is a registered observer.
+ * @param observer A {@link Watcher} that might be registered
+ * @return true if the observer is registered with this {@link Watchable}.
+ */
+ @Override
+ public boolean isRegisteredObserver(@NonNull Watcher observer) {
+ return mWatchable.isRegisteredObserver(observer);
+ }
+
+ /**
* Invokes {@link Watcher#onChange} on each registered observer. The method can be called
* with the {@link Watchable} that generated the event. In a tree of {@link Watchable}s, this
* is generally the first (deepest) {@link Watchable} to detect a change.
@@ -544,6 +554,8 @@
mRenamedPackages.registerObserver(mObserver);
mDefaultBrowserApp.registerObserver(mObserver);
mNextAppLinkGeneration.registerObserver(mObserver);
+
+ Watchable.verifyWatchedAttributes(this, mObserver);
}
Settings(File dataDir, RuntimePermissionsPersistence runtimePermissionsPersistence,
@@ -574,7 +586,6 @@
mStoppedPackagesFilename = new File(mSystemDir, "packages-stopped.xml");
mBackupStoppedPackagesFilename = new File(mSystemDir, "packages-stopped-backup.xml");
-
mPackages.registerObserver(mObserver);
mInstallerPackages.registerObserver(mObserver);
mKernelMapping.registerObserver(mObserver);
@@ -591,6 +602,8 @@
mRenamedPackages.registerObserver(mObserver);
mDefaultBrowserApp.registerObserver(mObserver);
mNextAppLinkGeneration.registerObserver(mObserver);
+
+ Watchable.verifyWatchedAttributes(this, mObserver);
}
/**
diff --git a/services/core/java/com/android/server/utils/Watchable.java b/services/core/java/com/android/server/utils/Watchable.java
index 7c99274..f936693 100644
--- a/services/core/java/com/android/server/utils/Watchable.java
+++ b/services/core/java/com/android/server/utils/Watchable.java
@@ -18,6 +18,10 @@
import android.annotation.NonNull;
import android.annotation.Nullable;
+import android.os.Build;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
/**
* Notify registered {@link Watcher}s when the content changes.
@@ -41,6 +45,13 @@
public void unregisterObserver(@NonNull Watcher observer);
/**
+ * Return true if the {@link Watcher) is a registered observer.
+ * @param observer A {@link Watcher} that might be registered
+ * @return true if the observer is registered with this {@link Watchable}.
+ */
+ public boolean isRegisteredObserver(@NonNull Watcher observer);
+
+ /**
* Invokes {@link Watcher#onChange} on each registered observer. The method can be called
* with the {@link Watchable} that generated the event. In a tree of {@link Watchable}s, this
* is generally the first (deepest) {@link Watchable} to detect a change.
@@ -48,4 +59,42 @@
* @param what The {@link Watchable} that generated the event.
*/
public void dispatchChange(@Nullable Watchable what);
+
+ /**
+ * Return true if the field is tagged with @Watched
+ */
+ private static boolean isWatched(Field f) {
+ for (Annotation a : f.getDeclaredAnnotations()) {
+ if (a.annotationType().equals(Watched.class)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Verify that all @Watched {@link Watchable} attributes are being watched by this
+ * class. This requires reflection and only runs in engineering or user debug
+ * builds.
+ */
+ static void verifyWatchedAttributes(Object base, Watcher observer) {
+ if (Build.IS_ENG || Build.IS_USERDEBUG) {
+ for (Field f : base.getClass().getDeclaredFields()) {
+ try {
+ final boolean flagged = isWatched(f);
+ final Object o = f.get(base);
+ final boolean watchable = o instanceof Watchable;
+ if (flagged && watchable) {
+ Watchable attr = (Watchable) f.get(base);
+ if (attr != null && !attr.isRegisteredObserver(observer)) {
+ throw new RuntimeException(f.getName() + " missing an observer");
+ }
+ }
+ } catch (IllegalAccessException e) {
+ // The field is protected; ignore it. Other exceptions that may be thrown by
+ // Field.get() are allowed to roll up.
+ }
+ }
+ }
+ }
}
diff --git a/services/core/java/com/android/server/utils/WatchableImpl.java b/services/core/java/com/android/server/utils/WatchableImpl.java
index d17fca1..527db54 100644
--- a/services/core/java/com/android/server/utils/WatchableImpl.java
+++ b/services/core/java/com/android/server/utils/WatchableImpl.java
@@ -66,6 +66,18 @@
}
/**
+ * Return true if the {@link Watcher) is a registered observer.
+ * @param observer A {@link Watcher} that might be registered
+ * @return true if the observer is registered with this {@link Watchable}.
+ */
+ @Override
+ public boolean isRegisteredObserver(@NonNull Watcher observer) {
+ synchronized (mObservers) {
+ return mObservers.contains(observer);
+ }
+ }
+
+ /**
* Return the number of registered observers.
*
* @return The number of registered observers.
diff --git a/services/core/java/com/android/server/utils/Watched.java b/services/core/java/com/android/server/utils/Watched.java
index d4a68ee..4340d01 100644
--- a/services/core/java/com/android/server/utils/Watched.java
+++ b/services/core/java/com/android/server/utils/Watched.java
@@ -27,6 +27,6 @@
* TODO(b/176923052) Automate validation of @Watchable attributes.
*/
@Target({ ElementType.FIELD })
-@Retention(RetentionPolicy.CLASS)
+@Retention(RetentionPolicy.RUNTIME)
public @interface Watched {
}