Merge "Stop recording unsupported class loaders."

am: c90f30d1d9

Change-Id: Ie89804c75a652608481e70bcf27b95ced43a578c

services/core/java/com/android/server/pm/dex/DexManager.java

@@ -30,6 +30,7 @@ import android.os.storage.StorageManager;
 import android.os.SystemProperties;
 import android.os.UserHandle;
 import android.provider.Settings.Global;
+import android.util.Log;
 import android.util.Slog;
 import android.util.jar.StrictJarFile;
 
@@ -74,7 +75,7 @@ public class DexManager {
     private static final String PROPERTY_NAME_PM_DEXOPT_PRIV_APPS_OOB_LIST =
             "pm.dexopt.priv-apps-oob-list";
 
-    private static final boolean DEBUG = false;
+    private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG);
 
     private final Context mContext;
 
@@ -192,6 +193,16 @@ public class DexManager {
         String[] classLoaderContexts = DexoptUtils.processContextForDexLoad(
                 classLoaderNames, classPaths);
 
+        // A null classLoaderContexts means that there are unsupported class loaders in the
+        // chain.
+        if (classLoaderContexts == null) {
+            if (DEBUG) {
+                Slog.i(TAG, loadingAppInfo.packageName +
+                        " uses unsupported class loader in " + classLoaderNames);
+            }
+            return;
+        }
+
         int dexPathIndex = 0;
         for (String dexPath : dexPathsToRegister) {
             // Find the owning package name.
@@ -219,14 +230,10 @@ public class DexManager {
                 }
 
                 // Record dex file usage. If the current usage is a new pattern (e.g. new secondary,
-                // or UsedBytOtherApps), record will return true and we trigger an async write
+                // or UsedByOtherApps), record will return true and we trigger an async write
                 // to disk to make sure we don't loose the data in case of a reboot.
 
-                // A null classLoaderContexts means that there are unsupported class loaders in the
+                String classLoaderContext = classLoaderContexts[dexPathIndex];
-                // chain.
-                String classLoaderContext = classLoaderContexts == null
-                        ? PackageDexUsage.UNSUPPORTED_CLASS_LOADER_CONTEXT
-                        : classLoaderContexts[dexPathIndex];
                 if (mPackageDexUsage.record(searchResult.mOwningPackageName,
                         dexPath, loaderUserId, loaderIsa, isUsedByOtherApps, primaryOrSplit,
                         loadingAppInfo.packageName, classLoaderContext)) {

services/core/java/com/android/server/pm/dex/PackageDexUsage.java

@@ -21,6 +21,7 @@ import android.util.Slog;
 import android.os.Build;
 
 import com.android.internal.annotations.GuardedBy;
+import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.FastPrintWriter;
 import com.android.server.pm.AbstractStatsBase;
 import com.android.server.pm.PackageManagerServiceUtils;
@@ -78,14 +79,16 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
     // skip optimizations on that dex files.
     /*package*/ static final String VARIABLE_CLASS_LOADER_CONTEXT =
             "=VariableClassLoaderContext=";
-    // The marker used for unsupported class loader contexts.
-    /*package*/ static final String UNSUPPORTED_CLASS_LOADER_CONTEXT =
-            "=UnsupportedClassLoaderContext=";
     // The markers used for unknown class loader contexts. This can happen if the dex file was
     // recorded in a previous version and we didn't have a chance to update its usage.
     /*package*/ static final String UNKNOWN_CLASS_LOADER_CONTEXT =
             "=UnknownClassLoaderContext=";
 
+    // The marker used for unsupported class loader contexts (no longer written, may occur in old
+    // files so discarded on read).
+    private static final String UNSUPPORTED_CLASS_LOADER_CONTEXT =
+            "=UnsupportedClassLoaderContext=";
+
     // Map which structures the information we have on a package.
     // Maps package name to package data (which stores info about UsedByOtherApps and
     // secondary dex files.).
@@ -365,6 +368,12 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
                 Set<String> loadingPackages = maybeReadLoadingPackages(in, version);
                 String classLoaderContext = maybeReadClassLoaderContext(in, version);
 
+                if (UNSUPPORTED_CLASS_LOADER_CONTEXT.equals(classLoaderContext)) {
+                    // We used to record use of unsupported class loaders, but we no longer do.
+                    // Discard such entries; they will be deleted when we next write the file.
+                    continue;
+                }
+
                 int ownerUserId = Integer.parseInt(elems[0]);
                 boolean isUsedByOtherApps = readBoolean(elems[1]);
                 DexUseInfo dexUseInfo = new DexUseInfo(isUsedByOtherApps, ownerUserId,
@@ -709,13 +718,13 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
         //      the compiled code will be private.
         private boolean mUsedByOtherAppsBeforeUpgrade;
 
-        public PackageUseInfo() {
+        /*package*/ PackageUseInfo() {
             mCodePathsUsedByOtherApps = new HashMap<>();
             mDexUseInfoMap = new HashMap<>();
         }
 
         // Creates a deep copy of the `other`.
-        public PackageUseInfo(PackageUseInfo other) {
+        private PackageUseInfo(PackageUseInfo other) {
             mCodePathsUsedByOtherApps = new HashMap<>();
             for (Map.Entry<String, Set<String>> e : other.mCodePathsUsedByOtherApps.entrySet()) {
                 mCodePathsUsedByOtherApps.put(e.getKey(), new HashSet<>(e.getValue()));
@@ -796,8 +805,9 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
         // Packages who load this dex file.
         private final Set<String> mLoadingPackages;
 
-        public DexUseInfo(boolean isUsedByOtherApps, int ownerUserId, String classLoaderContext,
+        @VisibleForTesting
-                String loaderIsa) {
+        /* package */ DexUseInfo(boolean isUsedByOtherApps, int ownerUserId,
+                String classLoaderContext, String loaderIsa) {
             mIsUsedByOtherApps = isUsedByOtherApps;
             mOwnerUserId = ownerUserId;
             mClassLoaderContext = classLoaderContext;
@@ -809,7 +819,7 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
         }
 
         // Creates a deep copy of the `other`.
-        public DexUseInfo(DexUseInfo other) {
+        private DexUseInfo(DexUseInfo other) {
             mIsUsedByOtherApps = other.mIsUsedByOtherApps;
             mOwnerUserId = other.mOwnerUserId;
             mClassLoaderContext = other.mClassLoaderContext;
@@ -827,11 +837,7 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
             if (UNKNOWN_CLASS_LOADER_CONTEXT.equals(mClassLoaderContext)) {
                 // Can happen if we read a previous version.
                 mClassLoaderContext = dexUseInfo.mClassLoaderContext;
-            } else if (UNSUPPORTED_CLASS_LOADER_CONTEXT.equals(dexUseInfo.mClassLoaderContext)) {
+            } else if (!Objects.equals(mClassLoaderContext, dexUseInfo.mClassLoaderContext)) {
-                // We detected an unsupported context.
-                mClassLoaderContext = UNSUPPORTED_CLASS_LOADER_CONTEXT;
-            } else if (!UNSUPPORTED_CLASS_LOADER_CONTEXT.equals(mClassLoaderContext) &&
-                    !Objects.equals(mClassLoaderContext, dexUseInfo.mClassLoaderContext)) {
                 // We detected a context change.
                 mClassLoaderContext = VARIABLE_CLASS_LOADER_CONTEXT;
             }
@@ -846,7 +852,7 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
             return mIsUsedByOtherApps;
         }
 
-        public int getOwnerUserId() {
+        /* package */ int getOwnerUserId() {
             return mOwnerUserId;
         }
 
@@ -860,17 +866,15 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
 
         public String getClassLoaderContext() { return mClassLoaderContext; }
 
-        public boolean isUnsupportedClassLoaderContext() {
+        @VisibleForTesting
-            return UNSUPPORTED_CLASS_LOADER_CONTEXT.equals(mClassLoaderContext);
+        /* package */ boolean isUnknownClassLoaderContext() {
-        }
-
-        public boolean isUnknownClassLoaderContext() {
             // The class loader context may be unknown if we loaded the data from a previous version
             // which didn't save the context.
             return UNKNOWN_CLASS_LOADER_CONTEXT.equals(mClassLoaderContext);
         }
 
-        public boolean isVariableClassLoaderContext() {
+        @VisibleForTesting
+        /* package */ boolean isVariableClassLoaderContext() {
             return VARIABLE_CLASS_LOADER_CONTEXT.equals(mClassLoaderContext);
         }
     }

services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java

@@ -402,15 +402,7 @@ public class DexManagerTests {
         List<String> secondaries = mBarUser0UnsupportedClassLoader.getSecondaryDexPaths();
         notifyDexLoad(mBarUser0UnsupportedClassLoader, secondaries, mUser0);
 
-        PackageUseInfo pui = getPackageUseInfo(mBarUser0UnsupportedClassLoader);
+        assertNoUseInfo(mBarUser0UnsupportedClassLoader);
-        assertIsUsedByOtherApps(mBarUser0UnsupportedClassLoader, pui, false);
-        assertEquals(secondaries.size(), pui.getDexUseInfoMap().size());
-        // We expect that all the contexts are unsupported.
-        String[] expectedContexts =
-                Collections.nCopies(secondaries.size(),
-                        PackageDexUsage.UNSUPPORTED_CLASS_LOADER_CONTEXT).toArray(new String[0]);
-        assertSecondaryUse(mBarUser0UnsupportedClassLoader, pui, secondaries,
-                /*isUsedByOtherApps*/false, mUser0, expectedContexts);
     }
 
     @Test
@@ -439,27 +431,18 @@ public class DexManagerTests {
     }
 
     @Test
-    public void testNotifyUnsupportedClassLoaderDoesNotChange() {
+    public void testNotifyUnsupportedClassLoaderDoesNotChangeExisting() {
-        List<String> secondaries = mBarUser0UnsupportedClassLoader.getSecondaryDexPaths();
+        List<String> secondaries = mBarUser0.getSecondaryDexPaths();
+
+        notifyDexLoad(mBarUser0, secondaries, mUser0);
+        PackageUseInfo pui = getPackageUseInfo(mBarUser0);
+        assertSecondaryUse(mBarUser0, pui, secondaries, /*isUsedByOtherApps*/false, mUser0);
+
+        // Record bar secondaries again with an unsupported class loader. This should not change the
+        // context.
         notifyDexLoad(mBarUser0UnsupportedClassLoader, secondaries, mUser0);
-
+        pui = getPackageUseInfo(mBarUser0);
-        PackageUseInfo pui = getPackageUseInfo(mBarUser0UnsupportedClassLoader);
+        assertSecondaryUse(mBarUser0, pui, secondaries, /*isUsedByOtherApps*/false, mUser0);
-        assertIsUsedByOtherApps(mBarUser0UnsupportedClassLoader, pui, false);
-        assertEquals(secondaries.size(), pui.getDexUseInfoMap().size());
-        // We expect that all the contexts are unsupported.
-        String[] expectedContexts =
-                Collections.nCopies(secondaries.size(),
-                        PackageDexUsage.UNSUPPORTED_CLASS_LOADER_CONTEXT).toArray(new String[0]);
-        assertSecondaryUse(mBarUser0UnsupportedClassLoader, pui, secondaries,
-                /*isUsedByOtherApps*/false, mUser0, expectedContexts);
-
-        // Record bar secondaries again with a different class loader. This will change the context.
-        // However, because the context was already marked as unsupported we should not chage it.
-        notifyDexLoad(mBarUser0DelegateLastClassLoader, secondaries, mUser0);
-        pui = getPackageUseInfo(mBarUser0UnsupportedClassLoader);
-        assertSecondaryUse(mBarUser0UnsupportedClassLoader, pui, secondaries,
-                /*isUsedByOtherApps*/false, mUser0, expectedContexts);
-
     }
 
     @Test

services/tests/servicestests/src/com/android/server/pm/dex/PackageDexUsageTests.java

@@ -397,20 +397,6 @@ public class PackageDexUsageTests {
         assertPackageDexUsage(null, expectedContext);
     }
 
-    @Test
-    public void testRecordClassLoaderContextUnsupportedContext() {
-        // Record a secondary dex file.
-        assertTrue(record(mFooSecondary1User0));
-        // Now update its context.
-        TestData unsupportedContext = mFooSecondary1User0.updateClassLoaderContext(
-                PackageDexUsage.UNSUPPORTED_CLASS_LOADER_CONTEXT);
-        assertTrue(record(unsupportedContext));
-
-        assertPackageDexUsage(null, unsupportedContext);
-        writeAndReadBack();
-        assertPackageDexUsage(null, unsupportedContext);
-    }
-
     @Test
     public void testRecordClassLoaderContextTransitionFromUnknown() {
         // Record a secondary dex file.
@@ -439,28 +425,40 @@ public class PackageDexUsageTests {
         PackageDexUsage.DexUseInfo validContext = new DexUseInfo(isUsedByOtherApps, userId,
                 "valid_context", "arm");
         assertFalse(validContext.isUnknownClassLoaderContext());
-        assertFalse(validContext.isUnsupportedClassLoaderContext());
         assertFalse(validContext.isVariableClassLoaderContext());
 
-        PackageDexUsage.DexUseInfo unsupportedContext = new DexUseInfo(isUsedByOtherApps, userId,
-                PackageDexUsage.UNSUPPORTED_CLASS_LOADER_CONTEXT, "arm");
-        assertFalse(unsupportedContext.isUnknownClassLoaderContext());
-        assertTrue(unsupportedContext.isUnsupportedClassLoaderContext());
-        assertFalse(unsupportedContext.isVariableClassLoaderContext());
-
         PackageDexUsage.DexUseInfo variableContext = new DexUseInfo(isUsedByOtherApps, userId,
                 PackageDexUsage.VARIABLE_CLASS_LOADER_CONTEXT, "arm");
         assertFalse(variableContext.isUnknownClassLoaderContext());
-        assertFalse(variableContext.isUnsupportedClassLoaderContext());
         assertTrue(variableContext.isVariableClassLoaderContext());
 
         PackageDexUsage.DexUseInfo unknownContext = new DexUseInfo(isUsedByOtherApps, userId,
                 PackageDexUsage.UNKNOWN_CLASS_LOADER_CONTEXT, "arm");
         assertTrue(unknownContext.isUnknownClassLoaderContext());
-        assertFalse(unknownContext.isUnsupportedClassLoaderContext());
         assertFalse(unknownContext.isVariableClassLoaderContext());
     }
 
+    @Test
+    public void testUnsupportedClassLoaderDiscardedOnRead() throws Exception {
+        String content = "PACKAGE_MANAGER__PACKAGE_DEX_USAGE__2\n"
+                + mBarSecondary1User0.mPackageName + "\n"
+                + "#" + mBarSecondary1User0.mDexFile + "\n"
+                + "0,0," + mBarSecondary1User0.mLoaderIsa + "\n"
+                + "@\n"
+                + "=UnsupportedClassLoaderContext=\n"
+
+                + mFooSecondary1User0.mPackageName + "\n"
+                + "#" + mFooSecondary1User0.mDexFile + "\n"
+                + "0,0," + mFooSecondary1User0.mLoaderIsa + "\n"
+                + "@\n"
+                + mFooSecondary1User0.mClassLoaderContext + "\n";
+
+        mPackageDexUsage.read(new StringReader(content));
+
+        assertPackageDexUsage(mFooBaseUser0, mFooSecondary1User0);
+        assertPackageDexUsage(mBarBaseUser0);
+    }
+
     @Test
     public void testReadVersion1() {
         String isa = VMRuntime.getInstructionSet(Build.SUPPORTED_ABIS[0]);