From f11786982a22e87f8e772f4e72d9af9a50f3b7b4 Mon Sep 17 00:00:00 2001 From: Lee Shombert Date: Thu, 7 Jan 2021 13:52:48 -0800 Subject: New watchables for PackageManagerService Bug: 161323622 Add WatchedArrayList, WatchedArraySet, and WatchedSparseIntArray to the server utilities. Test code is added to the frameworks test. The watched "collections" offer some consistent APIs. Every collection includes a copy constructor for itself and for the unwatched type. Every collection offers a copyIn() from, and copyOut() to, the unwatched type, to support runtime conversion. Add the Watched annotation interface. This annotation marks attributes that are watched for change detection; generally, any change invalidates a snapshot. It is also generally true that Watched attributes must be included in the snapshot. A bug in Snapshots.java is also fixed. Test: atest * FrameworksServicesTests:WatcherTest Change-Id: I9b6ad0b3f6a901e77c342c5a3e45c86bed510db6 --- .../java/com/android/server/pm/AppsFilter.java | 4 +- .../core/java/com/android/server/pm/Settings.java | 2 +- .../com/android/server/utils/WatchableImpl.java | 2 +- .../java/com/android/server/utils/Watched.java | 32 ++ .../com/android/server/utils/WatchedArrayList.java | 416 ++++++++++++++++++++ .../com/android/server/utils/WatchedArrayMap.java | 52 ++- .../com/android/server/utils/WatchedArraySet.java | 434 +++++++++++++++++++++ .../android/server/utils/WatchedSparseArray.java | 63 ++- .../server/utils/WatchedSparseBooleanArray.java | 76 +++- .../server/utils/WatchedSparseIntArray.java | 323 +++++++++++++++ .../server/pm/PackageManagerSettingsTests.java | 2 +- .../src/com/android/server/utils/WatcherTest.java | 349 ++++++++++++++++- 12 files changed, 1720 insertions(+), 35 deletions(-) create mode 100644 services/core/java/com/android/server/utils/Watched.java create mode 100644 services/core/java/com/android/server/utils/WatchedArrayList.java create mode 100644 services/core/java/com/android/server/utils/WatchedArraySet.java create mode 100644 services/core/java/com/android/server/utils/WatchedSparseIntArray.java diff --git a/services/core/java/com/android/server/pm/AppsFilter.java b/services/core/java/com/android/server/pm/AppsFilter.java index 094be0622bab..b76fff579918 100644 --- a/services/core/java/com/android/server/pm/AppsFilter.java +++ b/services/core/java/com/android/server/pm/AppsFilter.java @@ -443,7 +443,7 @@ public class AppsFilter implements Watchable, Snappable { } final StateProvider stateProvider = command -> { synchronized (injector.getLock()) { - command.currentState(injector.getSettings().getPackagesLocked().untrackedMap(), + command.currentState(injector.getSettings().getPackagesLocked().untrackedStorage(), injector.getUserManagerInternal().getUserInfos()); } }; @@ -979,7 +979,7 @@ public class AppsFilter implements Watchable, Snappable { @Nullable SparseArray getVisibilityAllowList(PackageSetting setting, int[] users, WatchedArrayMap existingSettings) { - return getVisibilityAllowList(setting, users, existingSettings.untrackedMap()); + return getVisibilityAllowList(setting, users, existingSettings.untrackedStorage()); } /** diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java index 7c4dadea89eb..f2aaee2e529f 100644 --- a/services/core/java/com/android/server/pm/Settings.java +++ b/services/core/java/com/android/server/pm/Settings.java @@ -471,7 +471,7 @@ public final class Settings implements Watchable, Snappable { private final File mSystemDir; public final KeySetManagerService mKeySetManagerService = - new KeySetManagerService(mPackages.untrackedMap()); + new KeySetManagerService(mPackages.untrackedStorage()); /** Settings and other information about permissions */ final LegacyPermissionSettings mPermissions; diff --git a/services/core/java/com/android/server/utils/WatchableImpl.java b/services/core/java/com/android/server/utils/WatchableImpl.java index 16400b186ab0..d17fca1d7a54 100644 --- a/services/core/java/com/android/server/utils/WatchableImpl.java +++ b/services/core/java/com/android/server/utils/WatchableImpl.java @@ -100,7 +100,7 @@ public class WatchableImpl implements Watchable { /** * Freeze the {@link Watchable}. - **/ + */ public void seal() { synchronized (mObservers) { mSealed = true; diff --git a/services/core/java/com/android/server/utils/Watched.java b/services/core/java/com/android/server/utils/Watched.java new file mode 100644 index 000000000000..d4a68ee735fd --- /dev/null +++ b/services/core/java/com/android/server/utils/Watched.java @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.utils; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Annotation type to mark an attribute that is monitored for change detection and + * snapshot creation. + * TODO(b/176923052) Automate validation of @Watchable attributes. + */ +@Target({ ElementType.FIELD }) +@Retention(RetentionPolicy.CLASS) +public @interface Watched { +} diff --git a/services/core/java/com/android/server/utils/WatchedArrayList.java b/services/core/java/com/android/server/utils/WatchedArrayList.java new file mode 100644 index 000000000000..bb0ba1329d86 --- /dev/null +++ b/services/core/java/com/android/server/utils/WatchedArrayList.java @@ -0,0 +1,416 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.utils; + +import android.annotation.NonNull; +import android.annotation.Nullable; + +import java.util.ArrayList; +import java.util.Collection; + +/** + * WatchedArrayMap is an {@link android.util.ArrayMap} that can report changes to itself. If its + * values are {@link Watchable} then the WatchedArrayMap will also report changes to the values. + * A {@link Watchable} is notified only once, no matter how many times it is stored in the array. + * @param The element type, stored in the array. + */ +public class WatchedArrayList extends WatchableImpl + implements Snappable { + + // The storage + private final ArrayList mStorage; + + // If true, the array is watching its children + private volatile boolean mWatching = false; + + // The local observer + private final Watcher mObserver = new Watcher() { + @Override + public void onChange(@Nullable Watchable what) { + WatchedArrayList.this.dispatchChange(what); + } + }; + + /** + * A convenience function called when the elements are added to or removed from the storage. + * The watchable is always {@link this}. + */ + private void onChanged() { + dispatchChange(this); + } + + /** + * A convenience function. Register the object if it is {@link Watchable} and if the + * array is currently watching. Note that the watching flag must be true if this + * function is to succeed. Also note that if this is called with the same object + * twice, is only registered once. + */ + private void registerChild(Object o) { + if (mWatching && o instanceof Watchable) { + ((Watchable) o).registerObserver(mObserver); + } + } + + /** + * A convenience function. Unregister the object if it is {@link Watchable} and if the + * array is currently watching. This unconditionally removes the object from the + * registered list. + */ + private void unregisterChild(Object o) { + if (mWatching && o instanceof Watchable) { + ((Watchable) o).unregisterObserver(mObserver); + } + } + + /** + * A convenience function. Unregister the object if it is {@link Watchable}, if the + * array is currently watching, and if there are no other instances of this object in + * the storage. Note that the watching flag must be true if this function is to + * succeed. The object must already have been removed from the storage before this + * method is called. + */ + private void unregisterChildIf(Object o) { + if (mWatching && o instanceof Watchable) { + if (!mStorage.contains(o)) { + ((Watchable) o).unregisterObserver(mObserver); + } + } + } + + /** + * Register a {@link Watcher} with the array. If this is the first Watcher than any + * array values that are {@link Watchable} are registered to the array itself. + */ + @Override + public void registerObserver(@NonNull Watcher observer) { + super.registerObserver(observer); + if (registeredObserverCount() == 1) { + // The watching flag must be set true before any children are registered. + mWatching = true; + final int end = mStorage.size(); + for (int i = 0; i < end; i++) { + registerChild(mStorage.get(i)); + } + } + } + + /** + * Unregister a {@link Watcher} from the array. If this is the last Watcher than any + * array values that are {@link Watchable} are unregistered to the array itself. + */ + @Override + public void unregisterObserver(@NonNull Watcher observer) { + super.unregisterObserver(observer); + if (registeredObserverCount() == 0) { + final int end = mStorage.size(); + for (int i = 0; i < end; i++) { + unregisterChild(mStorage.get(i)); + } + // The watching flag must be true while children are unregistered. + mWatching = false; + } + } + + /** + * Create a new empty {@link WatchedArrayList}. The default capacity of an array map + * is 0, and will grow once items are added to it. + */ + public WatchedArrayList() { + this(0); + } + + /** + * Create a new {@link WatchedArrayList} with a given initial capacity. + */ + public WatchedArrayList(int capacity) { + mStorage = new ArrayList(capacity); + } + + /** + * Create a new {@link WatchedArrayList} with the content of the collection. + */ + public WatchedArrayList(@Nullable Collection c) { + mStorage = new ArrayList(); + if (c != null) { + // There is no need to register children because the WatchedArrayList starts + // life unobserved. + mStorage.addAll(c); + } + } + + /** + * Create a {@link WatchedArrayList} from an {@link ArrayList} + */ + public WatchedArrayList(@NonNull ArrayList c) { + mStorage = new ArrayList<>(c); + } + + /** + * Create a {@link WatchedArrayList} from an {@link WatchedArrayList} + */ + public WatchedArrayList(@NonNull WatchedArrayList c) { + mStorage = new ArrayList<>(c.mStorage); + } + + /** + * Make a copy of src. Any data in is discarded. + */ + public void copyFrom(@NonNull ArrayList src) { + clear(); + final int end = src.size(); + mStorage.ensureCapacity(end); + for (int i = 0; i < end; i++) { + add(src.get(i)); + } + } + + /** + * Make dst a copy of . Any previous data in dst is discarded. + */ + public void copyTo(@NonNull ArrayList dst) { + dst.clear(); + final int end = size(); + dst.ensureCapacity(end); + for (int i = 0; i < end; i++) { + dst.add(get(i)); + } + } + + /** + * Return the underlying storage. This breaks the wrapper but is necessary when + * passing the array to distant methods. + */ + public ArrayList untrackedStorage() { + return mStorage; + } + + /** + * Append the specified element to the end of the list + */ + public boolean add(E value) { + final boolean result = mStorage.add(value); + registerChild(value); + onChanged(); + return result; + } + + /** + * Insert the element into the list + */ + public void add(int index, E value) { + mStorage.add(index, value); + registerChild(value); + onChanged(); + } + + /** + * Append the elements of the collection to the list. + */ + public boolean addAll(Collection c) { + if (c.size() > 0) { + for (E e: c) { + mStorage.add(e); + } + onChanged(); + return true; + } else { + return false; + } + } + + /** + * Insert the elements of the collection into the list at the index. + */ + public boolean addAll(int index, Collection c) { + if (c.size() > 0) { + for (E e: c) { + mStorage.add(index++, e); + } + onChanged(); + return true; + } else { + return false; + } + } + + + /** + * Remove all elements from the list. + */ + public void clear() { + // The storage cannot be simply cleared. Each element in the storage must be + // unregistered. Deregistration is only needed if the array is actually + // watching. + if (mWatching) { + final int end = mStorage.size(); + for (int i = 0; i < end; i++) { + unregisterChild(mStorage.get(i)); + } + } + mStorage.clear(); + onChanged(); + } + + /** + * Return true if the object is in the array. + */ + public boolean contains(Object o) { + return mStorage.contains(o); + } + + /** + * Ensure capacity. + */ + public void ensureCapacity(int min) { + mStorage.ensureCapacity(min); + } + + /** + * Retrieve the element at the specified index. + */ + public E get(int index) { + return mStorage.get(index); + } + + /** + * Return the index of the object. -1 is returned if the object is not in the list. + */ + public int indexOf(Object o) { + return mStorage.indexOf(o); + } + + /** + * True if the list has no elements + */ + public boolean isEmpty() { + return mStorage.isEmpty(); + } + + /** + * Return the index of the last occurrence of the object. + */ + public int lastIndexOf(Object o) { + return mStorage.lastIndexOf(o); + } + + /** + * Remove and return the element at the specified position. + */ + public E remove(int index) { + final E result = mStorage.remove(index); + unregisterChildIf(result); + onChanged(); + return result; + } + + /** + * Remove the first occurrence of the object in the list. Return true if the object + * was actually in the list and false otherwise. + */ + public boolean remove(Object o) { + if (mStorage.remove(o)) { + unregisterChildIf(o); + onChanged(); + return true; + } + return false; + } + + /** + * Replace the object at the index. + */ + public E set(int index, E value) { + final E result = mStorage.set(index, value); + if (value != result) { + unregisterChildIf(result); + registerChild(value); + onChanged(); + } + return result; + } + + /** + * Return the number of elements in the list. + */ + public int size() { + return mStorage.size(); + } + + /** + * {@inheritDoc} + */ + @Override + public boolean equals(@Nullable Object o) { + if (o instanceof WatchedArrayList) { + WatchedArrayList w = (WatchedArrayList) o; + return mStorage.equals(w.mStorage); + } else { + return false; + } + } + + /** + * {@inheritDoc} + */ + @Override + public int hashCode() { + return mStorage.hashCode(); + } + + /** + * Create a copy of the array. If the element is a subclass of Snapper then the copy + * contains snapshots of the elements. Otherwise the copy contains references to the + * elements. The returned snapshot is immutable. + * @return A new array whose elements are the elements of . + */ + public WatchedArrayList snapshot() { + WatchedArrayList l = new WatchedArrayList<>(size()); + snapshot(l, this); + return l; + } + + /** + * Make a snapshot of the argument. Note that is immutable when the + * method returns. must be empty when the function is called. + * @param r The source array, which is copied into + */ + public void snapshot(@NonNull WatchedArrayList r) { + snapshot(this, r); + } + + /** + * Make the destination a copy of the source. If the element is a subclass of Snapper then the + * copy contains snapshots of the elements. Otherwise the copy contains references to the + * elements. The destination must be initially empty. Upon return, the destination is + * immutable. + * @param dst The destination array. It must be empty. + * @param src The source array. It is not modified. + */ + public static void snapshot(@NonNull WatchedArrayList dst, + @NonNull WatchedArrayList src) { + if (dst.size() != 0) { + throw new IllegalArgumentException("snapshot destination is not empty"); + } + final int end = src.size(); + dst.mStorage.ensureCapacity(end); + for (int i = 0; i < end; i++) { + final E val = Snapshots.maybeSnapshot(src.get(i)); + dst.add(i, val); + } + dst.seal(); + } +} diff --git a/services/core/java/com/android/server/utils/WatchedArrayMap.java b/services/core/java/com/android/server/utils/WatchedArrayMap.java index e8065f140af7..7c1cde8502bd 100644 --- a/services/core/java/com/android/server/utils/WatchedArrayMap.java +++ b/services/core/java/com/android/server/utils/WatchedArrayMap.java @@ -159,11 +159,49 @@ public class WatchedArrayMap extends WatchableImpl } } + /** + * Create a {@link WatchedArrayMap} from an {@link ArrayMap} + */ + public WatchedArrayMap(@NonNull ArrayMap c) { + mStorage = new ArrayMap<>(c); + } + + /** + * Create a {@link WatchedArrayMap} from an {@link WatchedArrayMap} + */ + public WatchedArrayMap(@NonNull WatchedArrayMap c) { + mStorage = new ArrayMap<>(c.mStorage); + } + + /** + * Make a copy of src. Any data in is discarded. + */ + public void copyFrom(@NonNull ArrayMap src) { + clear(); + final int end = src.size(); + mStorage.ensureCapacity(end); + for (int i = 0; i < end; i++) { + put(src.keyAt(i), src.valueAt(i)); + } + } + + /** + * Make dst a copy of . Any previous data in dst is discarded. + */ + public void copyTo(@NonNull ArrayMap dst) { + dst.clear(); + final int end = size(); + dst.ensureCapacity(end); + for (int i = 0; i < end; i++) { + dst.put(keyAt(i), valueAt(i)); + } + } + /** * Return the underlying storage. This breaks the wrapper but is necessary when * passing the array to distant methods. */ - public ArrayMap untrackedMap() { + public ArrayMap untrackedStorage() { return mStorage; } @@ -213,7 +251,7 @@ public class WatchedArrayMap extends WatchableImpl * {@inheritDoc} */ @Override - public boolean equals(Object o) { + public boolean equals(@Nullable Object o) { if (o instanceof WatchedArrayMap) { WatchedArrayMap w = (WatchedArrayMap) o; return mStorage.equals(w.mStorage); @@ -400,6 +438,15 @@ public class WatchedArrayMap extends WatchableImpl return l; } + /** + * Make a snapshot of the argument. Note that is immutable when the + * method returns. must be empty when the function is called. + * @param r The source array, which is copied into + */ + public void snapshot(@NonNull WatchedArrayMap r) { + snapshot(this, r); + } + /** * Make the destination a copy of the source. If the element is a subclass of Snapper then the * copy contains snapshots of the elements. Otherwise the copy contains references to the @@ -414,6 +461,7 @@ public class WatchedArrayMap extends WatchableImpl throw new IllegalArgumentException("snapshot destination is not empty"); } final int end = src.size(); + dst.mStorage.ensureCapacity(end); for (int i = 0; i < end; i++) { final V val = Snapshots.maybeSnapshot(src.valueAt(i)); final K key = src.keyAt(i); diff --git a/services/core/java/com/android/server/utils/WatchedArraySet.java b/services/core/java/com/android/server/utils/WatchedArraySet.java new file mode 100644 index 000000000000..5070dd1675d3 --- /dev/null +++ b/services/core/java/com/android/server/utils/WatchedArraySet.java @@ -0,0 +1,434 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.utils; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.util.ArraySet; + +/** + * WatchedArraySet is an {@link android.util.ArraySet} that can report changes to itself. If its + * values are {@link Watchable} then the WatchedArraySet will also report changes to the values. + * A {@link Watchable} is notified only once, no matter how many times it is stored in the array. + * @param The element type + */ +public class WatchedArraySet extends WatchableImpl + implements Snappable { + + // The storage + private final ArraySet mStorage; + + // If true, the array is watching its children + private volatile boolean mWatching = false; + + // The local observer + private final Watcher mObserver = new Watcher() { + @Override + public void onChange(@Nullable Watchable what) { + WatchedArraySet.this.dispatchChange(what); + } + }; + + /** + * A convenience function called when the elements are added to or removed from the storage. + * The watchable is always {@link this}. + */ + private void onChanged() { + dispatchChange(this); + } + + /** + * A convenience function. Register the object if it is {@link Watchable} and if the + * array is currently watching. Note that the watching flag must be true if this + * function is to succeed. Also note that if this is called with the same object + * twice, is only registered once. + */ + private void registerChild(Object o) { + if (mWatching && o instanceof Watchable) { + ((Watchable) o).registerObserver(mObserver); + } + } + + /** + * A convenience function. Unregister the object if it is {@link Watchable} and if the + * array is currently watching. This unconditionally removes the object from the + * registered list. + */ + private void unregisterChild(Object o) { + if (mWatching && o instanceof Watchable) { + ((Watchable) o).unregisterObserver(mObserver); + } + } + + /** + * A convenience function. Unregister the object if it is {@link Watchable}, if the + * array is currently watching, and if there are no other instances of this object in + * the storage. Note that the watching flag must be true if this function is to + * succeed. The object must already have been removed from the storage before this + * method is called. + */ + private void unregisterChildIf(Object o) { + if (mWatching && o instanceof Watchable) { + if (!mStorage.contains(o)) { + ((Watchable) o).unregisterObserver(mObserver); + } + } + } + + /** + * Register a {@link Watcher} with the array. If this is the first Watcher than any + * array values that are {@link Watchable} are registered to the array itself. + */ + @Override + public void registerObserver(@NonNull Watcher observer) { + super.registerObserver(observer); + if (registeredObserverCount() == 1) { + // The watching flag must be set true before any children are registered. + mWatching = true; + final int end = mStorage.size(); + for (int i = 0; i < end; i++) { + registerChild(mStorage.valueAt(i)); + } + } + } + + /** + * Unregister a {@link Watcher} from the array. If this is the last Watcher than any + * array values that are {@link Watchable} are unregistered to the array itself. + */ + @Override + public void unregisterObserver(@NonNull Watcher observer) { + super.unregisterObserver(observer); + if (registeredObserverCount() == 0) { + final int end = mStorage.size(); + for (int i = 0; i < end; i++) { + unregisterChild(mStorage.valueAt(i)); + } + // The watching flag must be true while children are unregistered. + mWatching = false; + } + } + + /** + * Create a new empty {@link WatchedArraySet}. The default capacity of an array map + * is 0, and will grow once items are added to it. + */ + public WatchedArraySet() { + this(0, false); + } + + /** + * Create a new {@link WatchedArraySet} with a given initial capacity. + */ + public WatchedArraySet(int capacity) { + this(capacity, false); + } + + /** {@hide} */ + public WatchedArraySet(int capacity, boolean identityHashCode) { + mStorage = new ArraySet(capacity, identityHashCode); + } + + /** + * Create a new {@link WatchedArraySet} with items from the given array + */ + public WatchedArraySet(@Nullable E[] array) { + mStorage = new ArraySet(array); + } + + /** + * Create a {@link WatchedArraySet} from an {@link ArraySet} + */ + public WatchedArraySet(@NonNull ArraySet c) { + mStorage = new ArraySet<>(c); + } + + /** + * Create a {@link WatchedArraySet} from an {@link WatchedArraySet} + */ + public WatchedArraySet(@NonNull WatchedArraySet c) { + mStorage = new ArraySet<>(c.mStorage); + } + + /** + * Make a copy of src. Any data in is discarded. + */ + public void copyFrom(@NonNull ArraySet src) { + clear(); + final int end = src.size(); + mStorage.ensureCapacity(end); + for (int i = 0; i < end; i++) { + add(src.valueAt(i)); + } + } + + /** + * Make dst a copy of . Any previous data in dst is discarded. + */ + public void copyTo(@NonNull ArraySet dst) { + dst.clear(); + final int end = size(); + dst.ensureCapacity(end); + for (int i = 0; i < end; i++) { + dst.add(valueAt(i)); + } + } + + /** + * Return the underlying storage. This breaks the wrapper but is necessary when + * passing the array to distant methods. + */ + public ArraySet untrackedStorage() { + return mStorage; + } + + /** + * Make the array map empty. All storage is released. + */ + public void clear() { + // The storage cannot be simply cleared. Each element in the storage must be + // unregistered. Deregistration is only needed if the array is actually + // watching. + if (mWatching) { + final int end = mStorage.size(); + for (int i = 0; i < end; i++) { + unregisterChild(mStorage.valueAt(i)); + } + } + mStorage.clear(); + onChanged(); + } + + /** + * Check whether a value exists in the set. + * + * @param key The value to search for. + * @return Returns true if the value exists, else false. + */ + public boolean contains(Object key) { + return mStorage.contains(key); + } + + /** + * Returns the index of a value in the set. + * + * @param key The value to search for. + * @return Returns the index of the value if it exists, else a negative integer. + */ + public int indexOf(Object key) { + return mStorage.indexOf(key); + } + + /** + * Return the value at the given index in the array. + * + *

For indices outside of the range 0...size()-1, an + * {@link ArrayIndexOutOfBoundsException} is thrown.

+ * + * @param index The desired index, must be between 0 and {@link #size()}-1. + * @return Returns the value stored at the given index. + */ + public E valueAt(int index) { + return mStorage.valueAt(index); + } + + /** + * Return true if the array map contains no items. + */ + public boolean isEmpty() { + return mStorage.isEmpty(); + } + + /** + * Adds the specified object to this set. The set is not modified if it + * already contains the object. + * + * @param value the object to add. + * @return {@code true} if this set is modified, {@code false} otherwise. + */ + public boolean add(E value) { + final boolean result = mStorage.add(value); + registerChild(value); + onChanged(); + return result; + } + + /** + * Special fast path for appending items to the end of the array without validation. + * The array must already be large enough to contain the item. + * @hide + */ + public void append(E value) { + mStorage.append(value); + registerChild(value); + onChanged(); + } + + /** + * Perform a {@link #add(Object)} of all values in array + * @param array The array whose contents are to be retrieved. + */ + public void addAll(ArraySet array) { + final int end = array.size(); + for (int i = 0; i < end; i++) { + add(array.valueAt(i)); + } + } + + /** + * Perform a {@link #add(Object)} of all values in array + * @param array The array whose contents are to be retrieved. + */ + public void addAll(WatchedArraySet array) { + final int end = array.size(); + for (int i = 0; i < end; i++) { + add(array.valueAt(i)); + } + } + + /** + * Removes the specified object from this set. + * + * @param o the object to remove. + * @return {@code true} if this set was modified, {@code false} otherwise. + */ + public boolean remove(Object o) { + if (mStorage.remove(o)) { + unregisterChildIf(o); + onChanged(); + return true; + } + return false; + } + + /** + * Remove the key/value mapping at the given index. + * + *

For indices outside of the range 0...size()-1, an + * {@link ArrayIndexOutOfBoundsException} is thrown.

+ * + * @param index The desired index, must be between 0 and {@link #size()}-1. + * @return Returns the value that was stored at this index. + */ + public E removeAt(int index) { + final E result = mStorage.removeAt(index); + unregisterChildIf(result); + onChanged(); + return result; + } + + /** + * Perform a {@link #remove(Object)} of all values in array + * @param array The array whose contents are to be removed. + */ + public boolean removeAll(ArraySet array) { + final int end = array.size(); + boolean any = false; + for (int i = 0; i < end; i++) { + any = remove(array.valueAt(i)) || any; + } + return any; + } + + /** + * Return the number of items in this array map. + */ + public int size() { + return mStorage.size(); + } + + /** + * {@inheritDoc} + * + *

This implementation returns false if the object is not a set, or + * if the sets have different sizes. Otherwise, for each value in this + * set, it checks to make sure the value also exists in the other set. + * If any value doesn't exist, the method returns false; otherwise, it + * returns true. + */ + @Override + public boolean equals(@Nullable Object object) { + if (object instanceof WatchedArraySet) { + return mStorage.equals(((WatchedArraySet) object).mStorage); + } else { + return mStorage.equals(object); + } + } + + /** + * {@inheritDoc} + */ + @Override + public int hashCode() { + return mStorage.hashCode(); + } + + /** + * {@inheritDoc} + * + *

This implementation composes a string by iterating over its values. If + * this set contains itself as a value, the string "(this Set)" + * will appear in its place. + */ + @Override + public String toString() { + return mStorage.toString(); + } + + /** + * Create a copy of the array. If the element is a subclass of Snapper then the copy + * contains snapshots of the elements. Otherwise the copy contains references to the + * elements. The returned snapshot is immutable. + * @return A new array whose elements are the elements of . + */ + public WatchedArraySet snapshot() { + WatchedArraySet l = new WatchedArraySet<>(); + snapshot(l, this); + return l; + } + + /** + * Make a snapshot of the argument. Note that is immutable when the + * method returns. must be empty when the function is called. + * @param r The source array, which is copied into + */ + public void snapshot(@NonNull WatchedArraySet r) { + snapshot(this, r); + } + + /** + * Make the destination a copy of the source. If the element is a subclass of Snapper then the + * copy contains snapshots of the elements. Otherwise the copy contains references to the + * elements. The destination must be initially empty. Upon return, the destination is + * immutable. + * @param dst The destination array. It must be empty. + * @param src The source array. It is not modified. + */ + public static void snapshot(@NonNull WatchedArraySet dst, + @NonNull WatchedArraySet src) { + if (dst.size() != 0) { + throw new IllegalArgumentException("snapshot destination is not empty"); + } + final int end = src.size(); + dst.mStorage.ensureCapacity(end); + for (int i = 0; i < end; i++) { + final E val = Snapshots.maybeSnapshot(src.valueAt(i)); + dst.append(val); + } + dst.seal(); + } +} diff --git a/services/core/java/com/android/server/utils/WatchedSparseArray.java b/services/core/java/com/android/server/utils/WatchedSparseArray.java index 6797c6eb7801..9b99b9176d19 100644 --- a/services/core/java/com/android/server/utils/WatchedSparseArray.java +++ b/services/core/java/com/android/server/utils/WatchedSparseArray.java @@ -142,6 +142,13 @@ public class WatchedSparseArray extends WatchableImpl mStorage = new SparseArray(initialCapacity); } + /** + * Create a {@link WatchedSparseArray} from a {@link SparseArray} + */ + public WatchedSparseArray(@NonNull SparseArray c) { + mStorage = c.clone(); + } + /** * The copy constructor does not copy the watcher data. */ @@ -149,6 +156,36 @@ public class WatchedSparseArray extends WatchableImpl mStorage = r.mStorage.clone(); } + /** + * Make a copy of src. Any data in is discarded. + */ + public void copyFrom(@NonNull SparseArray src) { + clear(); + final int end = src.size(); + for (int i = 0; i < end; i++) { + put(src.keyAt(i), src.valueAt(i)); + } + } + + /** + * Make dst a copy of . Any previous data in dst is discarded. + */ + public void copyTo(@NonNull SparseArray dst) { + dst.clear(); + final int end = size(); + for (int i = 0; i < end; i++) { + dst.put(keyAt(i), valueAt(i)); + } + } + + /** + * Return the underlying storage. This breaks the wrapper but is necessary when + * passing the array to distant methods. + */ + public SparseArray untrackedStorage() { + return mStorage; + } + /** * Returns true if the key exists in the array. This is equivalent to * {@link #indexOfKey(int)} >= 0. @@ -390,6 +427,21 @@ public class WatchedSparseArray extends WatchableImpl onChanged(); } + @Override + public int hashCode() { + return mStorage.hashCode(); + } + + @Override + public boolean equals(@Nullable Object o) { + if (o instanceof WatchedSparseArray) { + WatchedSparseArray w = (WatchedSparseArray) o; + return mStorage.equals(w.mStorage); + } else { + return false; + } + } + /** *

This implementation composes a string by iterating over its mappings. If * this map contains itself as a value, the string "(this Map)" @@ -407,11 +459,20 @@ public class WatchedSparseArray extends WatchableImpl * @return A new array whose elements are the elements of . */ public WatchedSparseArray snapshot() { - WatchedSparseArray l = new WatchedSparseArray<>(); + WatchedSparseArray l = new WatchedSparseArray<>(size()); snapshot(l, this); return l; } + /** + * Make a snapshot of the argument. Note that is immutable when the + * method returns. must be empty when the function is called. + * @param r The source array, which is copied into + */ + public void snapshot(@NonNull WatchedSparseArray r) { + snapshot(this, r); + } + /** * Make the destination a copy of the source. If the element is a subclass of Snapper then the * copy contains snapshots of the elements. Otherwise the copy contains references to the diff --git a/services/core/java/com/android/server/utils/WatchedSparseBooleanArray.java b/services/core/java/com/android/server/utils/WatchedSparseBooleanArray.java index b845eea168a5..772a8d07cffb 100644 --- a/services/core/java/com/android/server/utils/WatchedSparseBooleanArray.java +++ b/services/core/java/com/android/server/utils/WatchedSparseBooleanArray.java @@ -17,6 +17,7 @@ package com.android.server.utils; import android.annotation.NonNull; +import android.annotation.Nullable; import android.util.SparseBooleanArray; /** @@ -52,6 +53,13 @@ public class WatchedSparseBooleanArray extends WatchableImpl mStorage = new SparseBooleanArray(initialCapacity); } + /** + * Create a {@link WatchedSparseBooleanArray} from a {@link SparseBooleanArray} + */ + public WatchedSparseBooleanArray(@NonNull SparseBooleanArray c) { + mStorage = c.clone(); + } + /** * The copy constructor does not copy the watcher data. */ @@ -59,6 +67,36 @@ public class WatchedSparseBooleanArray extends WatchableImpl mStorage = r.mStorage.clone(); } + /** + * Make a copy of src. Any data in is discarded. + */ + public void copyFrom(@NonNull SparseBooleanArray src) { + clear(); + final int end = src.size(); + for (int i = 0; i < end; i++) { + put(src.keyAt(i), src.valueAt(i)); + } + } + + /** + * Make dst a copy of . Any previous data in dst is discarded. + */ + public void copyTo(@NonNull SparseBooleanArray dst) { + dst.clear(); + final int end = size(); + for (int i = 0; i < end; i++) { + dst.put(keyAt(i), valueAt(i)); + } + } + + /** + * Return the underlying storage. This breaks the wrapper but is necessary when + * passing the array to distant methods. + */ + public SparseBooleanArray untrackedStorage() { + return mStorage; + } + /** * Gets the boolean mapped from the specified key, or false * if no such mapping has been made. @@ -99,10 +137,10 @@ public class WatchedSparseBooleanArray extends WatchableImpl * was one. */ public void put(int key, boolean value) { - if (mStorage.get(key) != value) { - mStorage.put(key, value); - onChanged(); - } + // There is no fast way to know if the key exists with the input value, so this + // method always notifies change listeners. + mStorage.put(key, value); + onChanged(); } /** @@ -219,8 +257,13 @@ public class WatchedSparseBooleanArray extends WatchableImpl } @Override - public boolean equals(Object that) { - return this == that || mStorage.equals(that); + public boolean equals(@Nullable Object o) { + if (o instanceof WatchedSparseBooleanArray) { + WatchedSparseBooleanArray w = (WatchedSparseBooleanArray) o; + return mStorage.equals(w.mStorage); + } else { + return false; + } } /** @@ -249,13 +292,26 @@ public class WatchedSparseBooleanArray extends WatchableImpl * @param r The source array, which is copied into */ public void snapshot(@NonNull WatchedSparseBooleanArray r) { - if (size() != 0) { + snapshot(this, r); + } + + /** + * Make the destination a copy of the source. If the element is a subclass of Snapper then the + * copy contains snapshots of the elements. Otherwise the copy contains references to the + * elements. The destination must be initially empty. Upon return, the destination is + * immutable. + * @param dst The destination array. It must be empty. + * @param src The source array. It is not modified. + */ + public static void snapshot(@NonNull WatchedSparseBooleanArray dst, + @NonNull WatchedSparseBooleanArray src) { + if (dst.size() != 0) { throw new IllegalArgumentException("snapshot destination is not empty"); } - final int end = r.size(); + final int end = src.size(); for (int i = 0; i < end; i++) { - put(r.keyAt(i), r.valueAt(i)); + dst.put(src.keyAt(i), src.valueAt(i)); } - seal(); + dst.seal(); } } diff --git a/services/core/java/com/android/server/utils/WatchedSparseIntArray.java b/services/core/java/com/android/server/utils/WatchedSparseIntArray.java new file mode 100644 index 000000000000..72705bf24199 --- /dev/null +++ b/services/core/java/com/android/server/utils/WatchedSparseIntArray.java @@ -0,0 +1,323 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.utils; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.util.SparseIntArray; + +/** + * A watched variant of SparseIntArray. Changes to the array are notified to + * registered {@link Watcher}s. + */ +public class WatchedSparseIntArray extends WatchableImpl + implements Snappable { + + // The storage + private final SparseIntArray mStorage; + + // A private convenience function + private void onChanged() { + dispatchChange(this); + } + + /** + * Creates a new WatchedSparseIntArray containing no mappings. + */ + public WatchedSparseIntArray() { + mStorage = new SparseIntArray(); + } + + /** + * Creates a new WatchedSparseIntArray containing no mappings that + * will not require any additional memory allocation to store the + * specified number of mappings. If you supply an initial capacity of + * 0, the sparse array will be initialized with a light-weight + * representation not requiring any additional array allocations. + */ + public WatchedSparseIntArray(int initialCapacity) { + mStorage = new SparseIntArray(initialCapacity); + } + + /** + * Create a {@link WatchedSparseIntArray} from a {@link SparseIntArray} + */ + public WatchedSparseIntArray(@NonNull SparseIntArray c) { + mStorage = c.clone(); + } + + /** + * The copy constructor does not copy the watcher data. + */ + public WatchedSparseIntArray(@NonNull WatchedSparseIntArray r) { + mStorage = r.mStorage.clone(); + } + + /** + * Make a copy of src. Any data in is discarded. + */ + public void copyFrom(@NonNull SparseIntArray src) { + clear(); + final int end = src.size(); + for (int i = 0; i < end; i++) { + put(src.keyAt(i), src.valueAt(i)); + } + } + + /** + * Make dst a copy of . Any previous data in dst is discarded. + */ + public void copyTo(@NonNull SparseIntArray dst) { + dst.clear(); + final int end = size(); + for (int i = 0; i < end; i++) { + dst.put(keyAt(i), valueAt(i)); + } + } + + /** + * Return the underlying storage. This breaks the wrapper but is necessary when + * passing the array to distant methods. + */ + public SparseIntArray untrackedStorage() { + return mStorage; + } + + /** + * Gets the boolean mapped from the specified key, or false + * if no such mapping has been made. + */ + public int get(int key) { + return mStorage.get(key); + } + + /** + * Gets the boolean mapped from the specified key, or the specified value + * if no such mapping has been made. + */ + public int get(int key, int valueIfKeyNotFound) { + return mStorage.get(key, valueIfKeyNotFound); + } + + /** + * Removes the mapping from the specified key, if there was any. + */ + public void delete(int key) { + // This code ensures that onChanged is called only if the key is actually + // present. + final int index = mStorage.indexOfKey(key); + if (index >= 0) { + mStorage.removeAt(index); + onChanged(); + } + } + + /** + * Removes the mapping at the specified index. + */ + public void removeAt(int index) { + mStorage.removeAt(index); + onChanged(); + } + + /** + * Adds a mapping from the specified key to the specified value, + * replacing the previous mapping from the specified key if there + * was one. + */ + public void put(int key, int value) { + // There is no fast way to know if the key exists with the input value, so this + // method always notifies change listeners. + mStorage.put(key, value); + onChanged(); + } + + /** + * Returns the number of key-value mappings that this SparseIntArray + * currently stores. + */ + public int size() { + return mStorage.size(); + } + + /** + * Given an index in the range 0...size()-1, returns + * the key from the indexth key-value mapping that this + * SparseIntArray stores. + * + *

The keys corresponding to indices in ascending order are guaranteed to + * be in ascending order, e.g., keyAt(0) will return the + * smallest key and keyAt(size()-1) will return the largest + * key.

+ * + *

For indices outside of the range 0...size()-1, the behavior is undefined for + * apps targeting {@link android.os.Build.VERSION_CODES#P} and earlier, and an + * {@link ArrayIndexOutOfBoundsException} is thrown for apps targeting + * {@link android.os.Build.VERSION_CODES#Q} and later.

+ */ + public int keyAt(int index) { + return mStorage.keyAt(index); + } + + /** + * Given an index in the range 0...size()-1, returns + * the value from the indexth key-value mapping that this + * SparseIntArray stores. + * + *

The values corresponding to indices in ascending order are guaranteed + * to be associated with keys in ascending order, e.g., + * valueAt(0) will return the value associated with the + * smallest key and valueAt(size()-1) will return the value + * associated with the largest key.

+ * + *

For indices outside of the range 0...size()-1, the behavior is undefined for + * apps targeting {@link android.os.Build.VERSION_CODES#P} and earlier, and an + * {@link ArrayIndexOutOfBoundsException} is thrown for apps targeting + * {@link android.os.Build.VERSION_CODES#Q} and later.

+ */ + public int valueAt(int index) { + return mStorage.valueAt(index); + } + + /** + * Directly set the value at a particular index. + * + *

For indices outside of the range 0...size()-1, the behavior is undefined for + * apps targeting {@link android.os.Build.VERSION_CODES#P} and earlier, and an + * {@link ArrayIndexOutOfBoundsException} is thrown for apps targeting + * {@link android.os.Build.VERSION_CODES#Q} and later.

+ */ + public void setValueAt(int index, int value) { + if (mStorage.valueAt(index) != value) { + mStorage.setValueAt(index, value); + onChanged(); + } + } + + /** + * Returns the index for which {@link #keyAt} would return the + * specified key, or a negative number if the specified + * key is not mapped. + */ + public int indexOfKey(int key) { + return mStorage.indexOfKey(key); + } + + /** + * Returns an index for which {@link #valueAt} would return the + * specified key, or a negative number if no keys map to the + * specified value. + * Beware that this is a linear search, unlike lookups by key, + * and that multiple keys can map to the same value and this will + * find only one of them. + */ + public int indexOfValue(int value) { + return mStorage.indexOfValue(value); + } + + /** + * Removes all key-value mappings from this SparseIntArray. + */ + public void clear() { + final int count = size(); + mStorage.clear(); + if (count > 0) { + onChanged(); + } + } + + /** + * Puts a key/value pair into the array, optimizing for the case where + * the key is greater than all existing keys in the array. + */ + public void append(int key, int value) { + mStorage.append(key, value); + onChanged(); + } + + /** + * Provides a copy of keys. + **/ + public int[] copyKeys() { + return mStorage.copyKeys(); + } + + @Override + public int hashCode() { + return mStorage.hashCode(); + } + + @Override + public boolean equals(@Nullable Object o) { + if (o instanceof WatchedSparseIntArray) { + WatchedSparseIntArray w = (WatchedSparseIntArray) o; + return mStorage.equals(w.mStorage); + } else { + return false; + } + } + + /** + * {@inheritDoc} + * + *

This implementation composes a string by iterating over its mappings. + */ + @Override + public String toString() { + return mStorage.toString(); + } + + /** + * Create a snapshot. The snapshot does not include any {@link Watchable} + * information. + */ + public WatchedSparseIntArray snapshot() { + WatchedSparseIntArray l = new WatchedSparseIntArray(this); + l.seal(); + return l; + } + + /** + * Make a snapshot of the argument. Note that is immutable when the + * method returns. must be empty when the function is called. + * @param r The source array, which is copied into + */ + public void snapshot(@NonNull WatchedSparseIntArray r) { + snapshot(this, r); + } + + /** + * Make the destination a copy of the source. If the element is a subclass of Snapper then the + * copy contains snapshots of the elements. Otherwise the copy contains references to the + * elements. The destination must be initially empty. Upon return, the destination is + * immutable. + * @param dst The destination array. It must be empty. + * @param src The source array. It is not modified. + */ + public static void snapshot(@NonNull WatchedSparseIntArray dst, + @NonNull WatchedSparseIntArray src) { + if (dst.size() != 0) { + throw new IllegalArgumentException("snapshot destination is not empty"); + } + final int end = src.size(); + for (int i = 0; i < end; i++) { + dst.put(src.keyAt(i), src.valueAt(i)); + } + dst.seal(); + } + +} diff --git a/services/tests/servicestests/src/com/android/server/pm/PackageManagerSettingsTests.java b/services/tests/servicestests/src/com/android/server/pm/PackageManagerSettingsTests.java index 282047afaa51..333ec9295b93 100644 --- a/services/tests/servicestests/src/com/android/server/pm/PackageManagerSettingsTests.java +++ b/services/tests/servicestests/src/com/android/server/pm/PackageManagerSettingsTests.java @@ -1215,7 +1215,7 @@ public class PackageManagerSettingsTests { private void verifyKeySetMetaData(Settings settings) throws ReflectiveOperationException, IllegalAccessException { ArrayMap packages = - settings.mPackages.untrackedMap(); + settings.mPackages.untrackedStorage(); KeySetManagerService ksms = settings.mKeySetManagerService; /* verify keyset and public key ref counts */ diff --git a/services/tests/servicestests/src/com/android/server/utils/WatcherTest.java b/services/tests/servicestests/src/com/android/server/utils/WatcherTest.java index 9bea9d4cedbd..7c65dc03a57e 100644 --- a/services/tests/servicestests/src/com/android/server/utils/WatcherTest.java +++ b/services/tests/servicestests/src/com/android/server/utils/WatcherTest.java @@ -20,12 +20,20 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import android.util.ArrayMap; +import android.util.ArraySet; +import android.util.SparseArray; +import android.util.SparseBooleanArray; +import android.util.SparseIntArray; + import androidx.test.filters.SmallTest; import org.junit.After; import org.junit.Before; import org.junit.Test; +import java.util.ArrayList; + /** * Test class for {@link Watcher}, {@link Watchable}, {@link WatchableImpl}, * {@link WatchedArrayMap}, {@link WatchedSparseArray}, and @@ -40,7 +48,7 @@ public class WatcherTest { // A counter to generate unique IDs for Leaf elements. private int mLeafId = 0; - // Useful indices used int the tests. + // Useful indices used in the tests. private static final int INDEX_A = 1; private static final int INDEX_B = 2; private static final int INDEX_C = 3; @@ -171,6 +179,7 @@ public class WatcherTest { @Test public void testWatchedArrayMap() { + final String name = "WatchedArrayMap"; WatchableTester tester; // Create a few leaves @@ -183,7 +192,7 @@ public class WatcherTest { WatchedArrayMap array = new WatchedArrayMap<>(); array.put(INDEX_A, leafA); array.put(INDEX_B, leafB); - tester = new WatchableTester(array, "WatchedArrayMap"); + tester = new WatchableTester(array, name); tester.verify(0, "Initial array - no registration"); leafA.tick(); tester.verify(0, "Updates with no registration"); @@ -231,20 +240,20 @@ public class WatcherTest { final WatchedArrayMap arraySnap = array.snapshot(); tester.verify(14, "Generate snapshot (no changes)"); // Verify that the snapshot is a proper copy of the source. - assertEquals("WatchedArrayMap snap same size", + assertEquals(name + " snap same size", array.size(), arraySnap.size()); for (int i = 0; i < array.size(); i++) { for (int j = 0; j < arraySnap.size(); j++) { - assertTrue("WatchedArrayMap elements differ", + assertTrue(name + " elements differ", array.valueAt(i) != arraySnap.valueAt(j)); } - assertTrue("WatchedArrayMap element copy", + assertTrue(name + " element copy", array.valueAt(i).equals(arraySnap.valueAt(i))); } leafD.tick(); tester.verify(15, "Tick after snapshot"); // Verify that the snapshot is sealed - verifySealed("WatchedArrayMap", ()->arraySnap.put(INDEX_A, leafA)); + verifySealed(name, ()->arraySnap.put(INDEX_A, leafA)); } // Recreate the snapshot since the test corrupted it. { @@ -253,10 +262,235 @@ public class WatcherTest { final Leaf arraySnapElement = arraySnap.valueAt(0); verifySealed("ArraySnapshotElement", ()->arraySnapElement.tick()); } + // Verify copy-in/out + { + final String msg = name + " copy-in/out failed"; + ArrayMap base = new ArrayMap<>(); + array.copyTo(base); + WatchedArrayMap copy = new WatchedArrayMap<>(); + copy.copyFrom(base); + if (!array.equals(copy)) { + fail(msg); + } + } + } + + @Test + public void testWatchedArraySet() { + final String name = "WatchedArraySet"; + WatchableTester tester; + + // Create a few leaves + Leaf leafA = new Leaf(); + Leaf leafB = new Leaf(); + Leaf leafC = new Leaf(); + Leaf leafD = new Leaf(); + + // Test WatchedArraySet + WatchedArraySet array = new WatchedArraySet<>(); + array.add(leafA); + array.add(leafB); + tester = new WatchableTester(array, name); + tester.verify(0, "Initial array - no registration"); + leafA.tick(); + tester.verify(0, "Updates with no registration"); + tester.register(); + tester.verify(0, "Updates with no registration"); + leafA.tick(); + tester.verify(1, "Updates with registration"); + leafB.tick(); + tester.verify(2, "Updates with registration"); + array.remove(leafB); + tester.verify(3, "Removed b"); + leafB.tick(); + tester.verify(3, "Updates with b not watched"); + array.add(leafB); + array.add(leafB); + tester.verify(5, "Added b once"); + leafB.tick(); + tester.verify(6, "Changed b - single notification"); + array.remove(leafB); + tester.verify(7, "Removed b"); + leafB.tick(); + tester.verify(7, "Changed b - not watched"); + array.remove(leafB); + tester.verify(7, "Removed non-existent b"); + array.clear(); + tester.verify(8, "Cleared array"); + leafA.tick(); + tester.verify(8, "Change to a not in array"); + + // Special methods + array.add(leafA); + array.add(leafB); + array.add(leafC); + tester.verify(11, "Added a, b, c"); + leafC.tick(); + tester.verify(12, "Ticked c"); + array.removeAt(array.indexOf(leafC)); + tester.verify(13, "Removed c"); + leafC.tick(); + tester.verify(13, "Ticked c, not registered"); + array.append(leafC); + tester.verify(14, "Append c"); + leafC.tick(); + leafD.tick(); + tester.verify(15, "Ticked d and c"); + assertEquals("Verify three elements", 3, array.size()); + + // Snapshot + { + final WatchedArraySet arraySnap = array.snapshot(); + tester.verify(15, "Generate snapshot (no changes)"); + // Verify that the snapshot is a proper copy of the source. + assertEquals(name + " snap same size", + array.size(), arraySnap.size()); + for (int i = 0; i < array.size(); i++) { + for (int j = 0; j < arraySnap.size(); j++) { + assertTrue(name + " elements differ", + array.valueAt(i) != arraySnap.valueAt(j)); + } + } + leafC.tick(); + tester.verify(16, "Tick after snapshot"); + // Verify that the array snapshot is sealed + verifySealed(name, ()->arraySnap.add(leafB)); + } + // Recreate the snapshot since the test corrupted it. + { + final WatchedArraySet arraySnap = array.snapshot(); + // Verify that elements are also snapshots + final Leaf arraySnapElement = arraySnap.valueAt(0); + verifySealed(name + " snap element", ()->arraySnapElement.tick()); + } + // Verify copy-in/out + { + final String msg = name + " copy-in/out"; + ArraySet base = new ArraySet<>(); + array.copyTo(base); + WatchedArraySet copy = new WatchedArraySet<>(); + copy.copyFrom(base); + if (!array.equals(copy)) { + fail(msg); + } + } + } + + @Test + public void testWatchedArrayList() { + final String name = "WatchedArrayList"; + WatchableTester tester; + + // Create a few leaves + Leaf leafA = new Leaf(); + Leaf leafB = new Leaf(); + Leaf leafC = new Leaf(); + Leaf leafD = new Leaf(); + + // Redefine the indices used in the tests to be zero-based + final int indexA = 0; + final int indexB = 1; + final int indexC = 2; + final int indexD = 3; + + // Test WatchedArrayList + WatchedArrayList array = new WatchedArrayList<>(); + // A spacer that takes up index 0 (and is not Watchable). + array.add(indexA, leafA); + array.add(indexB, leafB); + tester = new WatchableTester(array, name); + tester.verify(0, "Initial array - no registration"); + leafA.tick(); + tester.verify(0, "Updates with no registration"); + tester.register(); + tester.verify(0, "Updates with no registration"); + leafA.tick(); + tester.verify(1, "Updates with registration"); + leafB.tick(); + tester.verify(2, "Updates with registration"); + array.remove(indexB); + tester.verify(3, "Removed b"); + leafB.tick(); + tester.verify(3, "Updates with b not watched"); + array.add(indexB, leafB); + array.add(indexC, leafB); + tester.verify(5, "Added b twice"); + leafB.tick(); + tester.verify(6, "Changed b - single notification"); + array.remove(indexC); + tester.verify(7, "Removed first b"); + leafB.tick(); + tester.verify(8, "Changed b - single notification"); + array.remove(indexB); + tester.verify(9, "Removed second b"); + leafB.tick(); + tester.verify(9, "Updated leafB - no change"); + array.clear(); + tester.verify(10, "Cleared array"); + leafB.tick(); + tester.verify(10, "Change to b not in array"); + + // Special methods + array.add(indexA, leafA); + array.add(indexB, leafB); + array.add(indexC, leafC); + tester.verify(13, "Added c"); + leafC.tick(); + tester.verify(14, "Ticked c"); + array.set(array.indexOf(leafC), leafD); + tester.verify(15, "Replaced c with d"); + leafC.tick(); + leafD.tick(); + tester.verify(16, "Ticked d and c (c not registered)"); + array.add(leafC); + tester.verify(17, "Append c"); + leafC.tick(); + leafD.tick(); + tester.verify(19, "Ticked d and c"); + + // Snapshot + { + final WatchedArrayList arraySnap = array.snapshot(); + tester.verify(19, "Generate snapshot (no changes)"); + // Verify that the snapshot is a proper copy of the source. + assertEquals(name + " snap same size", + array.size(), arraySnap.size()); + for (int i = 0; i < array.size(); i++) { + for (int j = 0; j < arraySnap.size(); j++) { + assertTrue(name + " elements differ", + array.get(i) != arraySnap.get(j)); + } + assertTrue(name + " element copy", + array.get(i).equals(arraySnap.get(i))); + } + leafD.tick(); + tester.verify(20, "Tick after snapshot"); + // Verify that the array snapshot is sealed + verifySealed(name, ()->arraySnap.add(indexA, leafB)); + } + // Recreate the snapshot since the test corrupted it. + { + final WatchedArrayList arraySnap = array.snapshot(); + // Verify that elements are also snapshots + final Leaf arraySnapElement = arraySnap.get(0); + verifySealed("ArraySnapshotElement", ()->arraySnapElement.tick()); + } + // Verify copy-in/out + { + final String msg = name + " copy-in/out"; + ArrayList base = new ArrayList<>(); + array.copyTo(base); + WatchedArrayList copy = new WatchedArrayList<>(); + copy.copyFrom(base); + if (!array.equals(copy)) { + fail(msg); + } + } } @Test public void testWatchedSparseArray() { + final String name = "WatchedSparseArray"; WatchableTester tester; // Create a few leaves @@ -269,7 +503,7 @@ public class WatcherTest { WatchedSparseArray array = new WatchedSparseArray<>(); array.put(INDEX_A, leafA); array.put(INDEX_B, leafB); - tester = new WatchableTester(array, "WatchedSparseArray"); + tester = new WatchableTester(array, name); tester.verify(0, "Initial array - no registration"); leafA.tick(); tester.verify(0, "Updates with no registration"); @@ -338,20 +572,20 @@ public class WatcherTest { final WatchedSparseArray arraySnap = array.snapshot(); tester.verify(22, "Generate snapshot (no changes)"); // Verify that the snapshot is a proper copy of the source. - assertEquals("WatchedSparseArray snap same size", + assertEquals(name + " snap same size", array.size(), arraySnap.size()); for (int i = 0; i < array.size(); i++) { for (int j = 0; j < arraySnap.size(); j++) { - assertTrue("WatchedSparseArray elements differ", + assertTrue(name + " elements differ", array.valueAt(i) != arraySnap.valueAt(j)); } - assertTrue("WatchedArrayMap element copy", + assertTrue(name + " element copy", array.valueAt(i).equals(arraySnap.valueAt(i))); } leafD.tick(); tester.verify(23, "Tick after snapshot"); // Verify that the array snapshot is sealed - verifySealed("WatchedSparseArray", ()->arraySnap.put(INDEX_A, leafB)); + verifySealed(name, ()->arraySnap.put(INDEX_A, leafB)); } // Recreate the snapshot since the test corrupted it. { @@ -360,15 +594,30 @@ public class WatcherTest { final Leaf arraySnapElement = arraySnap.valueAt(0); verifySealed("ArraySnapshotElement", ()->arraySnapElement.tick()); } + // Verify copy-in/out + { + final String msg = name + " copy-in/out"; + SparseArray base = new SparseArray<>(); + array.copyTo(base); + WatchedSparseArray copy = new WatchedSparseArray<>(); + copy.copyFrom(base); + final int end = array.size(); + assertTrue(msg + " size mismatch " + end + " " + copy.size(), end == copy.size()); + for (int i = 0; i < end; i++) { + final int key = array.keyAt(i); + assertTrue(msg, array.get(i) == copy.get(i)); + } + } } @Test public void testWatchedSparseBooleanArray() { + final String name = "WatchedSparseBooleanArray"; WatchableTester tester; // Test WatchedSparseBooleanArray WatchedSparseBooleanArray array = new WatchedSparseBooleanArray(); - tester = new WatchableTester(array, "WatchedSparseBooleanArray"); + tester = new WatchableTester(array, name); tester.verify(0, "Initial array - no registration"); array.put(INDEX_A, true); tester.verify(0, "Updates with no registration"); @@ -376,14 +625,10 @@ public class WatcherTest { tester.verify(0, "Updates with no registration"); array.put(INDEX_B, true); tester.verify(1, "Updates with registration"); - array.put(INDEX_B, true); - tester.verify(1, "Null update"); array.put(INDEX_B, false); array.put(INDEX_C, true); tester.verify(3, "Updates with registration"); // Special methods - array.put(INDEX_C, true); - tester.verify(3, "Added true, no change"); array.setValueAt(array.indexOfKey(INDEX_C), false); tester.verify(4, "Replaced true with false"); array.append(INDEX_D, true); @@ -403,7 +648,77 @@ public class WatcherTest { array.put(INDEX_D, false); tester.verify(6, "Tick after snapshot"); // Verify that the array is sealed - verifySealed("WatchedSparseBooleanArray", ()->arraySnap.put(INDEX_D, false)); + verifySealed(name, ()->arraySnap.put(INDEX_D, false)); + } + // Verify copy-in/out + { + final String msg = name + " copy-in/out"; + SparseBooleanArray base = new SparseBooleanArray(); + array.copyTo(base); + WatchedSparseBooleanArray copy = new WatchedSparseBooleanArray(); + copy.copyFrom(base); + final int end = array.size(); + assertTrue(msg + " size mismatch/2 " + end + " " + copy.size(), end == copy.size()); + for (int i = 0; i < end; i++) { + final int key = array.keyAt(i); + assertTrue(msg + " element", array.get(i) == copy.get(i)); + } + } + } + + @Test + public void testWatchedSparseIntArray() { + final String name = "WatchedSparseIntArray"; + WatchableTester tester; + + // Test WatchedSparseIntArray + WatchedSparseIntArray array = new WatchedSparseIntArray(); + tester = new WatchableTester(array, name); + tester.verify(0, "Initial array - no registration"); + array.put(INDEX_A, 1); + tester.verify(0, "Updates with no registration"); + tester.register(); + tester.verify(0, "Updates with no registration"); + array.put(INDEX_B, 2); + tester.verify(1, "Updates with registration"); + array.put(INDEX_B, 4); + array.put(INDEX_C, 5); + tester.verify(3, "Updates with registration"); + // Special methods + array.setValueAt(array.indexOfKey(INDEX_C), 7); + tester.verify(4, "Replaced 6 with 7"); + array.append(INDEX_D, 8); + tester.verify(5, "Append 8"); + + // Snapshot + { + WatchedSparseIntArray arraySnap = array.snapshot(); + tester.verify(5, "Generate snapshot"); + // Verify that the snapshot is a proper copy of the source. + assertEquals("WatchedSparseIntArray snap same size", + array.size(), arraySnap.size()); + for (int i = 0; i < array.size(); i++) { + assertEquals(name + " element copy", + array.valueAt(i), arraySnap.valueAt(i)); + } + array.put(INDEX_D, 9); + tester.verify(6, "Tick after snapshot"); + // Verify that the array is sealed + verifySealed(name, ()->arraySnap.put(INDEX_D, 10)); + } + // Verify copy-in/out + { + final String msg = name + " copy-in/out"; + SparseIntArray base = new SparseIntArray(); + array.copyTo(base); + WatchedSparseIntArray copy = new WatchedSparseIntArray(); + copy.copyFrom(base); + final int end = array.size(); + assertTrue(msg + " size mismatch " + end + " " + copy.size(), end == copy.size()); + for (int i = 0; i < end; i++) { + final int key = array.keyAt(i); + assertTrue(msg, array.get(i) == copy.get(i)); + } } } } -- cgit v1.2.3-59-g8ed1b From a6e588bf9b1f1a35d19efbdd3e9ae2460c30084d Mon Sep 17 00:00:00 2001 From: Lee Shombert Date: Thu, 7 Jan 2021 13:52:51 -0800 Subject: PackageManager lock reduction: Settings on-change Bug: 161323622 Improve change detection in the Settings class. Attributes used in the snapshot are now watched. Test: atest * FrameworksServicesTests:UserSystemPackageInstallerTest * FrameworksServicesTests:PackageManagerSettingsTests * FrameworksServicesTests:PackageManagerServiceTest * FrameworksServicesTests:AppsFilterTest * FrameworksServicesTests:PackageInstallerSessionTest * FrameworksServicesTests:ScanTests * FrameworksServicesTests:WatcherTest Change-Id: Icc4a81900d2014e09cabbb2628702c6c20fc4da6 --- .../core/java/com/android/server/pm/Settings.java | 98 ++++++++++++++++------ 1 file changed, 74 insertions(+), 24 deletions(-) diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java index f2aaee2e529f..2d41a687833f 100644 --- a/services/core/java/com/android/server/pm/Settings.java +++ b/services/core/java/com/android/server/pm/Settings.java @@ -115,12 +115,15 @@ import com.android.server.pm.permission.LegacyPermissionSettings; import com.android.server.pm.permission.LegacyPermissionState; import com.android.server.pm.permission.LegacyPermissionState.PermissionState; import com.android.server.utils.Snappable; -import com.android.server.utils.Snapshots; import com.android.server.utils.TimingsTraceAndSlog; import com.android.server.utils.Watchable; import com.android.server.utils.WatchableImpl; +import com.android.server.utils.Watched; +import com.android.server.utils.WatchedArrayList; import com.android.server.utils.WatchedArrayMap; +import com.android.server.utils.WatchedArraySet; import com.android.server.utils.WatchedSparseArray; +import com.android.server.utils.WatchedSparseIntArray; import com.android.server.utils.Watcher; import libcore.io.IoUtils; @@ -350,6 +353,7 @@ public final class Settings implements Watchable, Snappable { private final File mKernelMappingFilename; /** Map from package name to settings */ + @Watched @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE) final WatchedArrayMap mPackages = new WatchedArrayMap<>(); @@ -357,21 +361,29 @@ public final class Settings implements Watchable, Snappable { * List of packages that were involved in installing other packages, i.e. are listed * in at least one app's InstallSource. */ - private final ArraySet mInstallerPackages = new ArraySet<>(); + @Watched + private final WatchedArraySet mInstallerPackages = new WatchedArraySet<>(); /** Map from package name to appId and excluded userids */ - private final ArrayMap mKernelMapping = new ArrayMap<>(); + @Watched + private final WatchedArrayMap mKernelMapping = + new WatchedArrayMap<>(); // List of replaced system applications + @Watched @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE) - final ArrayMap mDisabledSysPackages = new ArrayMap<>(); + final WatchedArrayMap mDisabledSysPackages = new WatchedArrayMap<>(); /** List of packages that are blocked for uninstall for specific users */ - private final SparseArray> mBlockUninstallPackages = new SparseArray<>(); + @Watched + private final WatchedSparseArray> mBlockUninstallPackages = + new WatchedSparseArray<>(); // Set of restored intent-filter verification states - private final ArrayMap mRestoredIntentFilterVerifications = - new ArrayMap(); + @Watched + private final WatchedArrayMap + mRestoredIntentFilterVerifications = + new WatchedArrayMap(); private static final class KernelPackageState { int appId; @@ -381,7 +393,8 @@ public final class Settings implements Watchable, Snappable { private static int mFirstAvailableUid = 0; /** Map from volume UUID to {@link VersionInfo} */ - private ArrayMap mVersion = new ArrayMap<>(); + @Watched + private WatchedArrayMap mVersion = new WatchedArrayMap<>(); /** * Version details for a storage volume that may hold apps. @@ -423,21 +436,27 @@ public final class Settings implements Watchable, Snappable { // The user's preferred activities associated with particular intent // filters. + @Watched private final WatchedSparseArray mPreferredActivities = new WatchedSparseArray<>(); // The persistent preferred activities of the user's profile/device owner // associated with particular intent filters. + @Watched private final WatchedSparseArray mPersistentPreferredActivities = new WatchedSparseArray<>(); // For every user, it is used to find to which other users the intent can be forwarded. + @Watched private final WatchedSparseArray mCrossProfileIntentResolvers = new WatchedSparseArray<>(); - final ArrayMap mSharedUsers = new ArrayMap<>(); - private final ArrayList mAppIds; - private final SparseArray mOtherAppIds; + @Watched + final WatchedArrayMap mSharedUsers = new WatchedArrayMap<>(); + @Watched + private final WatchedArrayList mAppIds; + @Watched + private final WatchedSparseArray mOtherAppIds; // For reading/writing settings file. private final ArrayList mPastSignatures = @@ -449,13 +468,17 @@ public final class Settings implements Watchable, Snappable { // Keys are the new names of the packages, values are the original // names. The packages appear everywhere else under their original // names. - private final ArrayMap mRenamedPackages = new ArrayMap(); + @Watched + private final WatchedArrayMap mRenamedPackages = + new WatchedArrayMap(); // For every user, it is used to find the package name of the default Browser App. - final SparseArray mDefaultBrowserApp = new SparseArray(); + @Watched + final WatchedSparseArray mDefaultBrowserApp = new WatchedSparseArray(); // App-link priority tracking, per-user - final SparseIntArray mNextAppLinkGeneration = new SparseIntArray(); + @Watched + final WatchedSparseIntArray mNextAppLinkGeneration = new WatchedSparseIntArray(); final StringBuilder mReadMessages = new StringBuilder(); @@ -492,8 +515,8 @@ public final class Settings implements Watchable, Snappable { public Settings(Map pkgSettings) { mLock = new Object(); mPackages.putAll(pkgSettings); - mAppIds = new ArrayList<>(); - mOtherAppIds = new SparseArray<>(); + mAppIds = new WatchedArrayList<>(); + mOtherAppIds = new WatchedSparseArray<>(); mSystemDir = null; mPermissions = null; mRuntimePermissionsPersistence = null; @@ -504,17 +527,30 @@ public final class Settings implements Watchable, Snappable { mStoppedPackagesFilename = null; mBackupStoppedPackagesFilename = null; mKernelMappingFilename = null; + mPackages.registerObserver(mObserver); + mInstallerPackages.registerObserver(mObserver); + mKernelMapping.registerObserver(mObserver); + mDisabledSysPackages.registerObserver(mObserver); + mBlockUninstallPackages.registerObserver(mObserver); + mRestoredIntentFilterVerifications.registerObserver(mObserver); + mVersion.registerObserver(mObserver); mPreferredActivities.registerObserver(mObserver); mPersistentPreferredActivities.registerObserver(mObserver); mCrossProfileIntentResolvers.registerObserver(mObserver); + mSharedUsers.registerObserver(mObserver); + mAppIds.registerObserver(mObserver); + mOtherAppIds.registerObserver(mObserver); + mRenamedPackages.registerObserver(mObserver); + mDefaultBrowserApp.registerObserver(mObserver); + mNextAppLinkGeneration.registerObserver(mObserver); } Settings(File dataDir, RuntimePermissionsPersistence runtimePermissionsPersistence, LegacyPermissionDataProvider permissionDataProvider, Object lock) { mLock = lock; - mAppIds = new ArrayList<>(); - mOtherAppIds = new SparseArray<>(); + mAppIds = new WatchedArrayList<>(); + mOtherAppIds = new WatchedSparseArray<>(); mPermissions = new LegacyPermissionSettings(lock); mRuntimePermissionsPersistence = new RuntimePermissionPersistence( runtimePermissionsPersistence); @@ -537,10 +573,24 @@ public final class Settings implements Watchable, Snappable { // Deprecated: Needed for migration mStoppedPackagesFilename = new File(mSystemDir, "packages-stopped.xml"); mBackupStoppedPackagesFilename = new File(mSystemDir, "packages-stopped-backup.xml"); + + mPackages.registerObserver(mObserver); + mInstallerPackages.registerObserver(mObserver); + mKernelMapping.registerObserver(mObserver); + mDisabledSysPackages.registerObserver(mObserver); + mBlockUninstallPackages.registerObserver(mObserver); + mRestoredIntentFilterVerifications.registerObserver(mObserver); + mVersion.registerObserver(mObserver); mPreferredActivities.registerObserver(mObserver); mPersistentPreferredActivities.registerObserver(mObserver); mCrossProfileIntentResolvers.registerObserver(mObserver); + mSharedUsers.registerObserver(mObserver); + mAppIds.registerObserver(mObserver); + mOtherAppIds.registerObserver(mObserver); + mRenamedPackages.registerObserver(mObserver); + mDefaultBrowserApp.registerObserver(mObserver); + mNextAppLinkGeneration.registerObserver(mObserver); } /** @@ -568,7 +618,7 @@ public final class Settings implements Watchable, Snappable { mInstallerPackages.addAll(r.mInstallerPackages); mKernelMapping.putAll(r.mKernelMapping); mDisabledSysPackages.putAll(r.mDisabledSysPackages); - Snapshots.copy(mBlockUninstallPackages, r.mBlockUninstallPackages); + mBlockUninstallPackages.snapshot(r.mBlockUninstallPackages); mRestoredIntentFilterVerifications.putAll(r.mRestoredIntentFilterVerifications); mVersion.putAll(r.mVersion); mVerifierDeviceIdentity = r.mVerifierDeviceIdentity; @@ -579,13 +629,13 @@ public final class Settings implements Watchable, Snappable { WatchedSparseArray.snapshot( mCrossProfileIntentResolvers, r.mCrossProfileIntentResolvers); mSharedUsers.putAll(r.mSharedUsers); - mAppIds = new ArrayList<>(r.mAppIds); - mOtherAppIds = r.mOtherAppIds.clone(); + mAppIds = r.mAppIds.snapshot(); + mOtherAppIds = r.mOtherAppIds.snapshot(); mPastSignatures.addAll(r.mPastSignatures); mKeySetRefs.putAll(r.mKeySetRefs); - mRenamedPackages.putAll(r.mRenamedPackages); - Snapshots.copy(mDefaultBrowserApp, r.mDefaultBrowserApp); - Snapshots.snapshot(mNextAppLinkGeneration, r.mNextAppLinkGeneration); + mRenamedPackages.snapshot(r.mRenamedPackages); + mDefaultBrowserApp.snapshot(r.mDefaultBrowserApp); + mNextAppLinkGeneration.snapshot(r.mNextAppLinkGeneration); // mReadMessages mPendingPackages.addAll(r.mPendingPackages); mSystemDir = null; -- cgit v1.2.3-59-g8ed1b From a1779c1ea38861694f3406c724d9975f20911f90 Mon Sep 17 00:00:00 2001 From: Lee Shombert Date: Thu, 7 Jan 2021 13:52:56 -0800 Subject: PackageManager lock reduction: IntentResolver Bug: 161323622 Address two comments on the IntentResolver: doCopy() is renamed to copyFrom() and WatchableIntentResolver.java is moved to be a peer of IntentResolver.java. Test: atest * FrameworksServicesTests:UserSystemPackageInstallerTest * FrameworksServicesTests:PackageManagerSettingsTests * FrameworksServicesTests:PackageManagerServiceTest * FrameworksServicesTests:AppsFilterTest * FrameworksServicesTests:PackageInstallerSessionTest * FrameworksServicesTests:ScanTests * FrameworksServicesTests:WatcherTest Change-Id: I0b17c3e8531e3e1324f9ca93516237297f810c1e --- .../java/com/android/server/IntentResolver.java | 12 ++- .../android/server/WatchableIntentResolver.java | 95 ++++++++++++++++++++++ .../server/pm/CrossProfileIntentResolver.java | 4 +- .../pm/PersistentPreferredIntentResolver.java | 4 +- .../android/server/pm/PreferredIntentResolver.java | 4 +- .../server/utils/WatchableIntentResolver.java | 93 --------------------- .../src/com/android/server/pm/AppsFilterTest.java | 75 ++++++++++++++++- 7 files changed, 184 insertions(+), 103 deletions(-) create mode 100644 services/core/java/com/android/server/WatchableIntentResolver.java delete mode 100644 services/core/java/com/android/server/utils/WatchableIntentResolver.java diff --git a/services/core/java/com/android/server/IntentResolver.java b/services/core/java/com/android/server/IntentResolver.java index ea1ac0c3fddc..2906ceebca58 100644 --- a/services/core/java/com/android/server/IntentResolver.java +++ b/services/core/java/com/android/server/IntentResolver.java @@ -839,14 +839,22 @@ public abstract class IntentResolver { } }; - // Make a copy of . The presumption is that is empty. - protected void doCopy(IntentResolver orig) { + // Make a copy of . The presumption is that is empty but all + // arrays are cleared out explicitly, just to be sure. + protected void copyFrom(IntentResolver orig) { + mFilters.clear(); mFilters.addAll(orig.mFilters); + mTypeToFilter.clear(); mTypeToFilter.putAll(orig.mTypeToFilter); + mBaseTypeToFilter.clear(); mBaseTypeToFilter.putAll(orig.mBaseTypeToFilter); + mWildTypeToFilter.clear(); mWildTypeToFilter.putAll(orig.mWildTypeToFilter); + mSchemeToFilter.clear(); mSchemeToFilter.putAll(orig.mSchemeToFilter); + mActionToFilter.clear(); mActionToFilter.putAll(orig.mActionToFilter); + mTypedActionToFilter.clear(); mTypedActionToFilter.putAll(orig.mTypedActionToFilter); } diff --git a/services/core/java/com/android/server/WatchableIntentResolver.java b/services/core/java/com/android/server/WatchableIntentResolver.java new file mode 100644 index 000000000000..3b5d168eba70 --- /dev/null +++ b/services/core/java/com/android/server/WatchableIntentResolver.java @@ -0,0 +1,95 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server; + +import android.annotation.NonNull; +import android.annotation.Nullable; + +import com.android.server.utils.Watchable; +import com.android.server.utils.WatchableImpl; +import com.android.server.utils.Watcher; + +import java.util.List; + +/** + * A watched {@link IntentResolver}. The parameters are inherited from the superclass. + * @param The filter type + * @param The resolver type. + * {@hide} + */ +public abstract class WatchableIntentResolver + extends IntentResolver + implements Watchable { + + /** + * Watchable machinery + */ + private final Watchable mWatchable = new WatchableImpl(); + /** + * Register an observer to receive change notifications. + * @param observer The observer to register. + */ + 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. + */ + public void unregisterObserver(@NonNull Watcher observer) { + mWatchable.unregisterObserver(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. + */ + public void dispatchChange(@Nullable Watchable what) { + mWatchable.dispatchChange(what); + } + /** + * Notify listeners that this object has changed. + */ + protected void onChanged() { + dispatchChange(this); + } + + @Override + public void addFilter(F f) { + super.addFilter(f); + onChanged(); + } + + @Override + public void removeFilter(F f) { + super.removeFilter(f); + onChanged(); + } + + @Override + protected void removeFilterInternal(F f) { + super.removeFilterInternal(f); + onChanged(); + } + + @Override + @SuppressWarnings("unchecked") + protected void sortResults(List results) { + super.sortResults(results); + onChanged(); + } +} diff --git a/services/core/java/com/android/server/pm/CrossProfileIntentResolver.java b/services/core/java/com/android/server/pm/CrossProfileIntentResolver.java index bf7f466457e9..aae6ce46de66 100644 --- a/services/core/java/com/android/server/pm/CrossProfileIntentResolver.java +++ b/services/core/java/com/android/server/pm/CrossProfileIntentResolver.java @@ -19,8 +19,8 @@ package com.android.server.pm; import android.annotation.NonNull; import android.content.IntentFilter; +import com.android.server.WatchableIntentResolver; import com.android.server.utils.Snappable; -import com.android.server.utils.WatchableIntentResolver; import java.util.List; @@ -57,7 +57,7 @@ class CrossProfileIntentResolver */ public CrossProfileIntentResolver snapshot() { CrossProfileIntentResolver result = new CrossProfileIntentResolver(); - result.doCopy(this); + result.copyFrom(this); return result; } } diff --git a/services/core/java/com/android/server/pm/PersistentPreferredIntentResolver.java b/services/core/java/com/android/server/pm/PersistentPreferredIntentResolver.java index d0f9787bacb8..c1bfcac50772 100644 --- a/services/core/java/com/android/server/pm/PersistentPreferredIntentResolver.java +++ b/services/core/java/com/android/server/pm/PersistentPreferredIntentResolver.java @@ -19,8 +19,8 @@ package com.android.server.pm; import android.annotation.NonNull; import android.content.IntentFilter; +import com.android.server.WatchableIntentResolver; import com.android.server.utils.Snappable; -import com.android.server.utils.WatchableIntentResolver; public class PersistentPreferredIntentResolver extends WatchableIntentResolver @@ -47,7 +47,7 @@ public class PersistentPreferredIntentResolver */ public PersistentPreferredIntentResolver snapshot() { PersistentPreferredIntentResolver result = new PersistentPreferredIntentResolver(); - result.doCopy(this); + result.copyFrom(this); return result; } } diff --git a/services/core/java/com/android/server/pm/PreferredIntentResolver.java b/services/core/java/com/android/server/pm/PreferredIntentResolver.java index b62421e31361..0e3b85ca677a 100644 --- a/services/core/java/com/android/server/pm/PreferredIntentResolver.java +++ b/services/core/java/com/android/server/pm/PreferredIntentResolver.java @@ -19,8 +19,8 @@ package com.android.server.pm; import android.annotation.NonNull; import android.content.IntentFilter; +import com.android.server.WatchableIntentResolver; import com.android.server.utils.Snappable; -import com.android.server.utils.WatchableIntentResolver; import java.io.PrintWriter; import java.util.ArrayList; @@ -76,7 +76,7 @@ public class PreferredIntentResolver */ public PreferredIntentResolver snapshot() { PreferredIntentResolver result = new PreferredIntentResolver(); - result.doCopy(this); + result.copyFrom(this); return result; } } diff --git a/services/core/java/com/android/server/utils/WatchableIntentResolver.java b/services/core/java/com/android/server/utils/WatchableIntentResolver.java deleted file mode 100644 index 767fc07d8cad..000000000000 --- a/services/core/java/com/android/server/utils/WatchableIntentResolver.java +++ /dev/null @@ -1,93 +0,0 @@ -/* - * Copyright (C) 2020 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.server.utils; - -import android.annotation.NonNull; -import android.annotation.Nullable; - -import com.android.server.IntentResolver; - -import java.util.List; - -/** - * A watched {@link IntentResolver}. The parameters are inherited from the superclass. - * @param The filter type - * @param The resolver type. - * {@hide} - */ -public abstract class WatchableIntentResolver - extends IntentResolver - implements Watchable { - - /** - * Watchable machinery - */ - private final Watchable mWatchable = new WatchableImpl(); - /** - * Register an observer to receive change notifications. - * @param observer The observer to register. - */ - 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. - */ - public void unregisterObserver(@NonNull Watcher observer) { - mWatchable.unregisterObserver(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. - */ - public void dispatchChange(@Nullable Watchable what) { - mWatchable.dispatchChange(what); - } - /** - * Notify listeners that this object has changed. - */ - protected void onChanged() { - dispatchChange(this); - } - - @Override - public void addFilter(F f) { - super.addFilter(f); - onChanged(); - } - - @Override - public void removeFilter(F f) { - super.removeFilter(f); - onChanged(); - } - - @Override - protected void removeFilterInternal(F f) { - super.removeFilterInternal(f); - onChanged(); - } - - @Override - @SuppressWarnings("unchecked") - protected void sortResults(List results) { - super.sortResults(results); - onChanged(); - } -} diff --git a/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java b/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java index 205548cc8d3c..9a52643b57f2 100644 --- a/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java @@ -209,7 +209,10 @@ public class AppsFilterTest { final AppsFilter appsFilter = new AppsFilter(mStateProvider, mFeatureConfigMock, new String[]{}, false, null, mMockExecutor); + final WatchableTester watcher = new WatchableTester(appsFilter, "onChange"); + watcher.register(); appsFilter.onSystemReady(); + watcher.verifyChangeReported("systemReady"); verify(mFeatureConfigMock).onSystemReady(); } @@ -218,45 +221,60 @@ public class AppsFilterTest { final AppsFilter appsFilter = new AppsFilter(mStateProvider, mFeatureConfigMock, new String[]{}, false, null, mMockExecutor); + final WatchableTester watcher = new WatchableTester(appsFilter, "onChange"); + watcher.register(); simulateAddBasicAndroid(appsFilter); + watcher.verifyChangeReported("addBasicAndroid"); appsFilter.onSystemReady(); + watcher.verifyChangeReported("systemReady"); PackageSetting target = simulateAddPackage(appsFilter, pkg("com.some.package", new IntentFilter("TEST_ACTION")), DUMMY_TARGET_APPID); + watcher.verifyChangeReported("add package"); PackageSetting calling = simulateAddPackage(appsFilter, pkg("com.some.other.package", new Intent("TEST_ACTION")), DUMMY_CALLING_APPID); + watcher.verifyChangeReported("add package"); assertFalse(appsFilter.shouldFilterApplication(DUMMY_CALLING_APPID, calling, target, SYSTEM_USER)); + watcher.verifyNoChangeReported("shouldFilterAplication"); } - @Test public void testQueriesProtectedAction_FilterDoesNotMatch() throws Exception { final AppsFilter appsFilter = new AppsFilter(mStateProvider, mFeatureConfigMock, new String[]{}, false, null, mMockExecutor); + final WatchableTester watcher = new WatchableTester(appsFilter, "onChange"); + watcher.register(); final Signature frameworkSignature = Mockito.mock(Signature.class); final PackageParser.SigningDetails frameworkSigningDetails = new PackageParser.SigningDetails(new Signature[]{frameworkSignature}, 1); final ParsingPackage android = pkg("android"); + watcher.verifyNoChangeReported("prepare"); android.addProtectedBroadcast("TEST_ACTION"); simulateAddPackage(appsFilter, android, 1000, b -> b.setSigningDetails(frameworkSigningDetails)); + watcher.verifyChangeReported("addPackage"); appsFilter.onSystemReady(); + watcher.verifyChangeReported("systemReady"); final int activityUid = DUMMY_TARGET_APPID; PackageSetting targetActivity = simulateAddPackage(appsFilter, pkg("com.target.activity", new IntentFilter("TEST_ACTION")), activityUid); + watcher.verifyChangeReported("addPackage"); final int receiverUid = DUMMY_TARGET_APPID + 1; PackageSetting targetReceiver = simulateAddPackage(appsFilter, pkgWithReceiver("com.target.receiver", new IntentFilter("TEST_ACTION")), receiverUid); + watcher.verifyChangeReported("addPackage"); final int callingUid = DUMMY_CALLING_APPID; PackageSetting calling = simulateAddPackage(appsFilter, pkg("com.calling.action", new Intent("TEST_ACTION")), callingUid); + watcher.verifyChangeReported("addPackage"); final int wildcardUid = DUMMY_CALLING_APPID + 1; PackageSetting callingWildCard = simulateAddPackage(appsFilter, pkg("com.calling.wildcard", new Intent("*")), wildcardUid); + watcher.verifyChangeReported("addPackage"); assertFalse(appsFilter.shouldFilterApplication(callingUid, calling, targetActivity, SYSTEM_USER)); @@ -267,6 +285,7 @@ public class AppsFilterTest { wildcardUid, callingWildCard, targetActivity, SYSTEM_USER)); assertTrue(appsFilter.shouldFilterApplication( wildcardUid, callingWildCard, targetReceiver, SYSTEM_USER)); + watcher.verifyNoChangeReported("shouldFilterApplication"); } @Test @@ -274,17 +293,24 @@ public class AppsFilterTest { final AppsFilter appsFilter = new AppsFilter(mStateProvider, mFeatureConfigMock, new String[]{}, false, null, mMockExecutor); + final WatchableTester watcher = new WatchableTester(appsFilter, "onChange"); + watcher.register(); simulateAddBasicAndroid(appsFilter); + watcher.verifyChangeReported("addPackage"); appsFilter.onSystemReady(); + watcher.verifyChangeReported("systemReady"); PackageSetting target = simulateAddPackage(appsFilter, pkgWithProvider("com.some.package", "com.some.authority"), DUMMY_TARGET_APPID); + watcher.verifyChangeReported("addPackage"); PackageSetting calling = simulateAddPackage(appsFilter, pkgQueriesProvider("com.some.other.package", "com.some.authority"), DUMMY_CALLING_APPID); + watcher.verifyChangeReported("addPackage"); assertFalse(appsFilter.shouldFilterApplication(DUMMY_CALLING_APPID, calling, target, SYSTEM_USER)); + watcher.verifyNoChangeReported("shouldFilterApplication"); } @Test @@ -292,17 +318,24 @@ public class AppsFilterTest { final AppsFilter appsFilter = new AppsFilter(mStateProvider, mFeatureConfigMock, new String[]{}, false, null, mMockExecutor); + final WatchableTester watcher = new WatchableTester(appsFilter, "onChange"); + watcher.register(); simulateAddBasicAndroid(appsFilter); + watcher.verifyChangeReported("addPackage"); appsFilter.onSystemReady(); + watcher.verifyChangeReported("systemReady"); PackageSetting target = simulateAddPackage(appsFilter, pkgWithProvider("com.some.package", "com.some.authority"), DUMMY_TARGET_APPID); + watcher.verifyChangeReported("addPackage"); PackageSetting calling = simulateAddPackage(appsFilter, pkgQueriesProvider("com.some.other.package", "com.some.other.authority"), DUMMY_CALLING_APPID); + watcher.verifyChangeReported("addPackage"); assertTrue(appsFilter.shouldFilterApplication(DUMMY_CALLING_APPID, calling, target, SYSTEM_USER)); + watcher.verifyNoChangeReported("shouldFilterApplication"); } @Test @@ -779,16 +812,23 @@ public class AppsFilterTest { final AppsFilter appsFilter = new AppsFilter(mStateProvider, mFeatureConfigMock, new String[]{}, false, null, mMockExecutor); + final WatchableTester watcher = new WatchableTester(appsFilter, "onChange"); + watcher.register(); simulateAddBasicAndroid(appsFilter); + watcher.verifyChangeReported("addBasicAndroid"); appsFilter.onSystemReady(); + watcher.verifyChangeReported("systemReady"); PackageSetting target = simulateAddPackage(appsFilter, pkg("com.some.package"), DUMMY_TARGET_APPID); + watcher.verifyChangeReported("add package"); PackageSetting calling = simulateAddPackage(appsFilter, pkg("com.some.other.package"), DUMMY_CALLING_APPID, withInstallSource(null, target.name, null, null, false)); + watcher.verifyChangeReported("add package"); assertTrue(appsFilter.shouldFilterApplication(DUMMY_CALLING_APPID, calling, target, SYSTEM_USER)); + watcher.verifyNoChangeReported("shouldFilterAplication"); } @Test @@ -796,16 +836,23 @@ public class AppsFilterTest { final AppsFilter appsFilter = new AppsFilter(mStateProvider, mFeatureConfigMock, new String[]{}, false, null, mMockExecutor); + final WatchableTester watcher = new WatchableTester(appsFilter, "onChange"); + watcher.register(); simulateAddBasicAndroid(appsFilter); + watcher.verifyChangeReported("addBasicAndroid"); appsFilter.onSystemReady(); + watcher.verifyChangeReported("systemReady"); PackageSetting target = simulateAddPackage(appsFilter, pkg("com.some.package"), DUMMY_TARGET_APPID); + watcher.verifyChangeReported("add package"); PackageSetting calling = simulateAddPackage(appsFilter, pkg("com.some.other.package"), DUMMY_CALLING_APPID, withInstallSource(null, null, target.name, null, false)); + watcher.verifyChangeReported("add package"); assertFalse(appsFilter.shouldFilterApplication(DUMMY_CALLING_APPID, calling, target, SYSTEM_USER)); + watcher.verifyNoChangeReported("shouldFilterAplication"); } @Test @@ -813,15 +860,20 @@ public class AppsFilterTest { final AppsFilter appsFilter = new AppsFilter(mStateProvider, mFeatureConfigMock, new String[]{}, false, null, mMockExecutor); + final WatchableTester watcher = new WatchableTester(appsFilter, "onChange"); + watcher.register(); simulateAddBasicAndroid(appsFilter); + watcher.verifyChangeReported("addBasicAndroid"); appsFilter.onSystemReady(); - + watcher.verifyChangeReported("systemReady"); PackageSetting target = simulateAddPackage(appsFilter, pkg("com.some.package"), DUMMY_TARGET_APPID); + watcher.verifyChangeReported("add package"); PackageSetting instrumentation = simulateAddPackage(appsFilter, pkgWithInstrumentation("com.some.other.package", "com.some.package"), DUMMY_CALLING_APPID); + watcher.verifyChangeReported("add package"); assertFalse( appsFilter.shouldFilterApplication(DUMMY_CALLING_APPID, instrumentation, target, @@ -829,6 +881,7 @@ public class AppsFilterTest { assertFalse( appsFilter.shouldFilterApplication(DUMMY_TARGET_APPID, target, instrumentation, SYSTEM_USER)); + watcher.verifyNoChangeReported("shouldFilterAplication"); } @Test @@ -836,8 +889,12 @@ public class AppsFilterTest { final AppsFilter appsFilter = new AppsFilter(mStateProvider, mFeatureConfigMock, new String[]{}, false, null, mMockExecutor); + final WatchableTester watcher = new WatchableTester(appsFilter, "onChange"); + watcher.register(); simulateAddBasicAndroid(appsFilter); + watcher.verifyChangeReported("addBasicAndroid"); appsFilter.onSystemReady(); + watcher.verifyChangeReported("systemReady"); final int systemAppId = Process.FIRST_APPLICATION_UID - 1; final int seesNothingAppId = Process.FIRST_APPLICATION_UID; @@ -845,25 +902,34 @@ public class AppsFilterTest { final int queriesProviderAppId = Process.FIRST_APPLICATION_UID + 2; PackageSetting system = simulateAddPackage(appsFilter, pkg("some.system.pkg"), systemAppId); + watcher.verifyChangeReported("add package"); PackageSetting seesNothing = simulateAddPackage(appsFilter, pkg("com.some.package"), seesNothingAppId); + watcher.verifyChangeReported("add package"); PackageSetting hasProvider = simulateAddPackage(appsFilter, pkgWithProvider("com.some.other.package", "com.some.authority"), hasProviderAppId); + watcher.verifyChangeReported("add package"); PackageSetting queriesProvider = simulateAddPackage(appsFilter, pkgQueriesProvider("com.yet.some.other.package", "com.some.authority"), queriesProviderAppId); + watcher.verifyChangeReported("add package"); final SparseArray systemFilter = appsFilter.getVisibilityAllowList(system, USER_ARRAY, mExisting); + watcher.verifyNoChangeReported("getVisibility"); assertThat(toList(systemFilter.get(SYSTEM_USER)), contains(seesNothingAppId, hasProviderAppId, queriesProviderAppId)); + watcher.verifyNoChangeReported("getVisibility"); final SparseArray seesNothingFilter = appsFilter.getVisibilityAllowList(seesNothing, USER_ARRAY, mExisting); + watcher.verifyNoChangeReported("getVisibility"); assertThat(toList(seesNothingFilter.get(SYSTEM_USER)), contains(seesNothingAppId)); + watcher.verifyNoChangeReported("getVisibility"); assertThat(toList(seesNothingFilter.get(SECONDARY_USER)), contains(seesNothingAppId)); + watcher.verifyNoChangeReported("getVisibility"); final SparseArray hasProviderFilter = appsFilter.getVisibilityAllowList(hasProvider, USER_ARRAY, mExisting); @@ -872,17 +938,22 @@ public class AppsFilterTest { SparseArray queriesProviderFilter = appsFilter.getVisibilityAllowList(queriesProvider, USER_ARRAY, mExisting); + watcher.verifyNoChangeReported("getVisibility"); assertThat(toList(queriesProviderFilter.get(SYSTEM_USER)), contains(queriesProviderAppId)); + watcher.verifyNoChangeReported("getVisibility"); // provider read appsFilter.grantImplicitAccess(hasProviderAppId, queriesProviderAppId); + watcher.verifyChangeReported("grantImplicitAccess"); // ensure implicit access is included in the filter queriesProviderFilter = appsFilter.getVisibilityAllowList(queriesProvider, USER_ARRAY, mExisting); + watcher.verifyNoChangeReported("getVisibility"); assertThat(toList(queriesProviderFilter.get(SYSTEM_USER)), contains(hasProviderAppId, queriesProviderAppId)); + watcher.verifyNoChangeReported("getVisibility"); } @Test -- cgit v1.2.3-59-g8ed1b From e5371006acbd46b9587963dbe85fa2fe02e9750e Mon Sep 17 00:00:00 2001 From: Lee Shombert Date: Thu, 7 Jan 2021 13:52:59 -0800 Subject: 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 --- .../android/server/WatchableIntentResolver.java | 17 ++++++++ .../java/com/android/server/pm/AppsFilter.java | 13 ++++++ .../com/android/server/pm/InstantAppRegistry.java | 3 ++ .../java/com/android/server/pm/SettingBase.java | 14 +++++++ .../core/java/com/android/server/pm/Settings.java | 15 ++++++- .../java/com/android/server/utils/Watchable.java | 49 ++++++++++++++++++++++ .../com/android/server/utils/WatchableImpl.java | 12 ++++++ .../java/com/android/server/utils/Watched.java | 2 +- 8 files changed, 123 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/WatchableIntentResolver.java b/services/core/java/com/android/server/WatchableIntentResolver.java index 3b5d168eba70..2ef94f17e5d9 100644 --- a/services/core/java/com/android/server/WatchableIntentResolver.java +++ b/services/core/java/com/android/server/WatchableIntentResolver.java @@ -39,28 +39,45 @@ public abstract class WatchableIntentResolver * 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 b76fff579918..f8990c065341 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 @@ public class AppsFilter implements Watchable, Snappable { * * @param observer The {@link Watcher} to be notified when the {@link Watchable} changes. */ + @Override public void registerObserver(@NonNull Watcher observer) { mWatchable.registerObserver(observer); } @@ -177,10 +178,21 @@ public class AppsFilter implements Watchable, Snappable { * * @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 @@ -188,6 +200,7 @@ public class AppsFilter implements Watchable, Snappable { * * @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 69d3e5c2f941..c3bca285dca3 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 @@ class InstantAppRegistry implements Watchable, Snappable { 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 7924d5d48876..0e8a278f3b6b 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 @@ public abstract class SettingBase implements Watchable { * * @param observer The {@link Watcher} to be notified when the {@link Watchable} changes. */ + @Override public void registerObserver(@NonNull Watcher observer) { mWatchable.registerObserver(observer); } @@ -59,10 +60,21 @@ public abstract class SettingBase implements Watchable { * * @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 @@ -70,9 +82,11 @@ public abstract class SettingBase implements Watchable { * * @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 2d41a687833f..50c1065a6000 100644 --- a/services/core/java/com/android/server/pm/Settings.java +++ b/services/core/java/com/android/server/pm/Settings.java @@ -191,6 +191,16 @@ public final class Settings implements Watchable, Snappable { 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 @@ -544,6 +554,8 @@ public final class Settings implements Watchable, Snappable { mRenamedPackages.registerObserver(mObserver); mDefaultBrowserApp.registerObserver(mObserver); mNextAppLinkGeneration.registerObserver(mObserver); + + Watchable.verifyWatchedAttributes(this, mObserver); } Settings(File dataDir, RuntimePermissionsPersistence runtimePermissionsPersistence, @@ -574,7 +586,6 @@ public final class Settings implements Watchable, Snappable { 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 @@ public final class Settings implements Watchable, Snappable { 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 7c99274f3df2..f936693bd621 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 @@ package com.android.server.utils; 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. @@ -40,6 +44,13 @@ public interface Watchable { */ 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 @@ -48,4 +59,42 @@ public interface Watchable { * @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 d17fca1d7a54..527db5402e74 100644 --- a/services/core/java/com/android/server/utils/WatchableImpl.java +++ b/services/core/java/com/android/server/utils/WatchableImpl.java @@ -65,6 +65,18 @@ public class WatchableImpl implements Watchable { } } + /** + * 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. * diff --git a/services/core/java/com/android/server/utils/Watched.java b/services/core/java/com/android/server/utils/Watched.java index d4a68ee735fd..4340d015119a 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 @@ import java.lang.annotation.Target; * TODO(b/176923052) Automate validation of @Watchable attributes. */ @Target({ ElementType.FIELD }) -@Retention(RetentionPolicy.CLASS) +@Retention(RetentionPolicy.RUNTIME) public @interface Watched { } -- cgit v1.2.3-59-g8ed1b