Revert "Do not load xml metadata for unchanged packages in RegisteredServicesCache"

This reverts commit 8094880025d0680ec4cf586929d85b9050d4e9d6.

Reason for revert: changed services order
Test: manual
Bug: 136261465

Change-Id: I2f78f0dbbace212309b87f779efad020cf255196
Merged-In: I2f78f0dbbace212309b87f779efad020cf255196

2 files changed, 20 insertions, 80 deletions

diff --git a/core/java/android/content/pm/RegisteredServicesCache.java b/core/java/android/content/pm/RegisteredServicesCache.java
index c8f046775211..020e8c22b128 100644
--- a/core/java/android/content/pm/RegisteredServicesCache.java
+++ b/core/java/android/content/pm/RegisteredServicesCache.java
@@ -17,7 +17,6 @@
 package android.content.pm;
 
 import android.Manifest;
-import android.annotation.Nullable;
 import android.content.BroadcastReceiver;
 import android.content.ComponentName;
 import android.content.Context;
@@ -175,8 +174,7 @@ public abstract class RegisteredServicesCache<V> {
         mContext.registerReceiver(mUserRemovedReceiver, userFilter);
     }
 
-    @VisibleForTesting
-    protected void handlePackageEvent(Intent intent, int userId) {
+    private final 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.
@@ -238,9 +236,6 @@ public abstract class RegisteredServicesCache<V> {
 
     public void invalidateCache(int userId) {
         synchronized (mServicesLock) {
-            if (DEBUG) {
-                Slog.d(TAG, "invalidating cache for " + userId + " " + mInterfaceName);
-            }
             final UserServices<V> user = findOrCreateUserLocked(userId);
             user.services = null;
             onServicesChangedLocked(userId);
@@ -465,48 +460,34 @@ public abstract class RegisteredServicesCache<V> {
      *                    or null to assume that everything is affected.
      * @param userId the user for whom to update the services map.
      */
-    private void generateServicesMap(@Nullable int[] changedUids, int userId) {
+    private void generateServicesMap(int[] changedUids, int userId) {
         if (DEBUG) {
             Slog.d(TAG, "generateServicesMap() for " + userId + ", changed UIDs = "
                     + Arrays.toString(changedUids));
         }
 
+        final ArrayList<ServiceInfo<V>> serviceInfos = new ArrayList<>();
+        final List<ResolveInfo> resolveInfos = queryIntentServices(userId);
+        for (ResolveInfo resolveInfo : resolveInfos) {
+            try {
+                ServiceInfo<V> 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<V> user = findOrCreateUserLocked(userId);
-            final boolean cacheInvalid = user.services == null;
-            if (cacheInvalid) {
+            final boolean firstScan = user.services == null;
+            if (firstScan) {
                 user.services = Maps.newHashMap();
             }
 
-            final ArrayList<ServiceInfo<V>> serviceInfos = new ArrayList<>();
-            final List<ResolveInfo> 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<V> 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<V> info : serviceInfos) {
@@ -527,7 +508,7 @@ public abstract class RegisteredServicesCache<V> {
                     changed = true;
                     user.services.put(info.type, info);
                     user.persistentServices.put(info.type, info.uid);
-                    if (!(user.mPersistentServicesFileDidNotExist && cacheInvalid)) {
+                    if (!(user.mPersistentServicesFileDidNotExist && firstScan)) {
                         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 c8b449e68da7..d3d1f22af3cb 100644
--- a/core/tests/coretests/src/android/content/pm/RegisteredServicesCacheTest.java
+++ b/core/tests/coretests/src/android/content/pm/RegisteredServicesCacheTest.java
@@ -16,7 +16,6 @@
 
 package android.content.pm;
 
-import android.content.Intent;
 import android.content.res.Resources;
 import android.os.FileUtils;
 import android.os.Parcel;
@@ -189,36 +188,6 @@ 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<TestServiceType> newServiceInfo(
             TestServiceType type, int uid) {
         final ComponentInfo info = new ComponentInfo();
@@ -296,11 +265,6 @@ 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);
         }
 
@@ -339,11 +303,6 @@ 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<TestServiceType> {