From 8094880025d0680ec4cf586929d85b9050d4e9d6 Mon Sep 17 00:00:00 2001 From: Ng Zhi An Date: Thu, 24 Jan 2019 13:45:57 -0800 Subject: Do not load xml metadata for unchanged packages in RegisteredServicesCache RegisteredServicesCache caches services in an xml, so the system doesn't have to scan all the apks. Whenever a package is changed (add/remove/replaced), the broadcast receiver in RegisteredServicesCache will refresh this cache by quering all matching services and reading their xml metadata. There is extra work done here, because only the changed package will have services added/removed. The edge case is after a cache is invalidated, we want to query package manager for everything, regardless of what changedUids is (we could have arrived here if invalidateCache is called, and then a package change event fires, before a getAllServices is called to rescan everything). Add a new test to verify that the optimization takes care of the case when the cache is invalidated. Bug: 117755076 Bug: 122912184 Test: atest RegisteredServicesCache Test: dumpsys content # check sync adapters Merged-In: I5c1f57108c4b67d24b198000d57216c63d35290a Change-Id: I5c1f57108c4b67d24b198000d57216c63d35290a --- .../content/pm/RegisteredServicesCache.java | 59 ++++++++++++++-------- .../content/pm/RegisteredServicesCacheTest.java | 41 +++++++++++++++ 2 files changed, 80 insertions(+), 20 deletions(-) diff --git a/core/java/android/content/pm/RegisteredServicesCache.java b/core/java/android/content/pm/RegisteredServicesCache.java index 020e8c22b128..c8f046775211 100644 --- a/core/java/android/content/pm/RegisteredServicesCache.java +++ b/core/java/android/content/pm/RegisteredServicesCache.java @@ -17,6 +17,7 @@ package android.content.pm; import android.Manifest; +import android.annotation.Nullable; import android.content.BroadcastReceiver; import android.content.ComponentName; import android.content.Context; @@ -174,7 +175,8 @@ public abstract class RegisteredServicesCache { mContext.registerReceiver(mUserRemovedReceiver, userFilter); } - private final void handlePackageEvent(Intent intent, int userId) { + @VisibleForTesting + protected void handlePackageEvent(Intent intent, int userId) { // Don't regenerate the services map when the package is removed or its // ASEC container unmounted as a step in replacement. The subsequent // _ADDED / _AVAILABLE call will regenerate the map in the final state. @@ -236,6 +238,9 @@ public abstract class RegisteredServicesCache { public void invalidateCache(int userId) { synchronized (mServicesLock) { + if (DEBUG) { + Slog.d(TAG, "invalidating cache for " + userId + " " + mInterfaceName); + } final UserServices user = findOrCreateUserLocked(userId); user.services = null; onServicesChangedLocked(userId); @@ -460,34 +465,48 @@ public abstract class RegisteredServicesCache { * or null to assume that everything is affected. * @param userId the user for whom to update the services map. */ - private void generateServicesMap(int[] changedUids, int userId) { + private void generateServicesMap(@Nullable int[] changedUids, int userId) { if (DEBUG) { Slog.d(TAG, "generateServicesMap() for " + userId + ", changed UIDs = " + Arrays.toString(changedUids)); } - final ArrayList> serviceInfos = new ArrayList<>(); - final List resolveInfos = queryIntentServices(userId); - for (ResolveInfo resolveInfo : resolveInfos) { - try { - ServiceInfo info = parseServiceInfo(resolveInfo); - if (info == null) { - Log.w(TAG, "Unable to load service info " + resolveInfo.toString()); - continue; - } - serviceInfos.add(info); - } catch (XmlPullParserException|IOException e) { - Log.w(TAG, "Unable to load service info " + resolveInfo.toString(), e); - } - } - synchronized (mServicesLock) { final UserServices user = findOrCreateUserLocked(userId); - final boolean firstScan = user.services == null; - if (firstScan) { + final boolean cacheInvalid = user.services == null; + if (cacheInvalid) { user.services = Maps.newHashMap(); } + final ArrayList> serviceInfos = new ArrayList<>(); + final List resolveInfos = queryIntentServices(userId); + + for (ResolveInfo resolveInfo : resolveInfos) { + try { + // when changedUids == null, we want to do a rescan of everything, this means + // it's the initial scan, and containsUid will trivially return true + // when changedUids != null, we got here because a package changed, but + // invalidateCache could have been called (thus user.services == null), and we + // should query from PackageManager again + if (!cacheInvalid + && !containsUid( + changedUids, resolveInfo.serviceInfo.applicationInfo.uid)) { + if (DEBUG) { + Slog.d(TAG, "Skipping parseServiceInfo for " + resolveInfo); + } + continue; + } + ServiceInfo info = parseServiceInfo(resolveInfo); + if (info == null) { + Log.w(TAG, "Unable to load service info " + resolveInfo.toString()); + continue; + } + serviceInfos.add(info); + } catch (XmlPullParserException | IOException e) { + Log.w(TAG, "Unable to load service info " + resolveInfo.toString(), e); + } + } + StringBuilder changes = new StringBuilder(); boolean changed = false; for (ServiceInfo info : serviceInfos) { @@ -508,7 +527,7 @@ public abstract class RegisteredServicesCache { changed = true; user.services.put(info.type, info); user.persistentServices.put(info.type, info.uid); - if (!(user.mPersistentServicesFileDidNotExist && firstScan)) { + if (!(user.mPersistentServicesFileDidNotExist && cacheInvalid)) { notifyListener(info.type, userId, false /* removed */); } } else if (previousUid == info.uid) { diff --git a/core/tests/coretests/src/android/content/pm/RegisteredServicesCacheTest.java b/core/tests/coretests/src/android/content/pm/RegisteredServicesCacheTest.java index d3d1f22af3cb..c8b449e68da7 100644 --- a/core/tests/coretests/src/android/content/pm/RegisteredServicesCacheTest.java +++ b/core/tests/coretests/src/android/content/pm/RegisteredServicesCacheTest.java @@ -16,6 +16,7 @@ package android.content.pm; +import android.content.Intent; import android.content.res.Resources; import android.os.FileUtils; import android.os.Parcel; @@ -188,6 +189,36 @@ public class RegisteredServicesCacheTest extends AndroidTestCase { assertEquals(0, cache.getPersistentServicesSize(u1)); } + /** + * Check that an optimization to skip a call to PackageManager handles an invalidated cache. + * + * We added an optimization in generateServicesMap to only query PackageManager for packages + * that have been changed, because if a package is unchanged, we have already cached the + * services info for it, so we can save a query to PackageManager (and save some memory). + * However, if invalidateCache was called, we cannot optimize, and must do a full query. + * The initial optimization was buggy because it failed to check for an invalidated cache, and + * only scanned the changed packages, given in the ACTION_PACKAGE_CHANGED intent (b/122912184). + */ + public void testParseServiceInfoOptimizationHandlesInvalidatedCache() { + TestServicesCache cache = new TestServicesCache(); + cache.addServiceForQuerying(U0, r1, newServiceInfo(t1, UID1)); + cache.addServiceForQuerying(U0, r2, newServiceInfo(t2, UID2)); + assertEquals(2, cache.getAllServicesSize(U0)); + + // simulate the client of the cache invalidating it + cache.invalidateCache(U0); + + // there should be 0 services (userServices.services == null ) at this point, but we don't + // call getAllServicesSize since that would force a full scan of packages, + // instead we trigger a package change in a package that is in the list of services + Intent intent = new Intent(Intent.ACTION_PACKAGE_CHANGED); + intent.putExtra(Intent.EXTRA_UID, UID1); + cache.handlePackageEvent(intent, U0); + + // check that the optimization does a full query and caches both services + assertEquals(2, cache.getAllServicesSize(U0)); + } + private static RegisteredServicesCache.ServiceInfo newServiceInfo( TestServiceType type, int uid) { final ComponentInfo info = new ComponentInfo(); @@ -265,6 +296,11 @@ public class RegisteredServicesCacheTest extends AndroidTestCase { map = new HashMap<>(); mServices.put(userId, map); } + // in actual cases, resolveInfo should always have a serviceInfo, since we specifically + // query for intent services + resolveInfo.serviceInfo = new android.content.pm.ServiceInfo(); + resolveInfo.serviceInfo.applicationInfo = + new ApplicationInfo(serviceInfo.componentInfo.applicationInfo); map.put(resolveInfo, serviceInfo); } @@ -303,6 +339,11 @@ public class RegisteredServicesCacheTest extends AndroidTestCase { public void onUserRemoved(int userId) { super.onUserRemoved(userId); } + + @Override + public void handlePackageEvent(Intent intent, int userId) { + super.handlePackageEvent(intent, userId); + } } static class TestSerializer implements XmlSerializerAndParser { -- cgit v1.2.3-59-g8ed1b