Merge "Always use the latest CLC for system server"

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

@@ -215,7 +215,7 @@ public class DexManager {
                         searchResult.mOutcome == DEX_SEARCH_FOUND_SPLIT;
 
                 if (primaryOrSplit && !isUsedByOtherApps
-                        && !PLATFORM_PACKAGE_NAME.equals(searchResult.mOwningPackageName)) {
+                        && !isPlatformPackage(searchResult.mOwningPackageName)) {
                     // If the dex file is the primary apk (or a split) and not isUsedByOtherApps
                     // do not record it. This case does not bring any new usable information
                     // and can be safely skipped.
@@ -232,15 +232,24 @@ public class DexManager {
                 }
 
                 String classLoaderContext = mapping.getValue();
+
+                // Overwrite the class loader context for system server (instead of merging it).
+                // We expect system server jars to only change contexts in between OTAs and to
+                // otherwise be stable.
+                // Instead of implementing a complex clear-context logic post OTA, it is much
+                // simpler to always override the context for system server. This way, the context
+                // will always be up to date and we will avoid merging which could lead to the
+                // the context being marked as variable and thus making dexopt non-optimal.
+                boolean overwriteCLC = isPlatformPackage(searchResult.mOwningPackageName);
+
                 if (classLoaderContext != null
                         && VMRuntime.isValidClassLoaderContext(classLoaderContext)) {
                     // Record dex file usage. If the current usage is a new pattern (e.g. new
                     // secondary, 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.
-
                     if (mPackageDexUsage.record(searchResult.mOwningPackageName,
                             dexPath, loaderUserId, loaderIsa, primaryOrSplit,
-                            loadingAppInfo.packageName, classLoaderContext)) {
+                            loadingAppInfo.packageName, classLoaderContext, overwriteCLC)) {
                         mPackageDexUsage.maybeWriteAsync();
                     }
                 }
@@ -474,7 +483,7 @@ public class DexManager {
      *         because they don't need to be compiled)..
      */
     public boolean dexoptSecondaryDex(DexoptOptions options) {
-        if (PLATFORM_PACKAGE_NAME.equals(options.getPackageName())) {
+        if (isPlatformPackage(options.getPackageName())) {
             // We could easily redirect to #dexoptSystemServer in this case. But there should be
             // no-one calling this method directly for system server.
             // As such we prefer to abort in this case.
@@ -534,7 +543,7 @@ public class DexManager {
      * <p>PackageDexOptimizer.DEX_OPT_PERFORMED if all dexopt operations succeeded.
      */
     public int dexoptSystemServer(DexoptOptions options) {
-        if (!PLATFORM_PACKAGE_NAME.equals(options.getPackageName())) {
+        if (!isPlatformPackage(options.getPackageName())) {
             Slog.wtf(TAG, "Non system server package used when trying to dexopt system server:"
                     + options.getPackageName());
             return PackageDexOptimizer.DEX_OPT_FAILED;
@@ -662,7 +671,7 @@ public class DexManager {
             // Special handle system server files.
             // We don't need an installd call because we have permissions to check if the file
             // exists.
-            if (PLATFORM_PACKAGE_NAME.equals(packageName)) {
+            if (isPlatformPackage(packageName)) {
                 if (!Files.exists(Paths.get(dexPath))) {
                     if (DEBUG) {
                         Slog.w(TAG, "A dex file previously loaded by System Server does not exist "
@@ -739,7 +748,8 @@ public class DexManager {
             boolean newUpdate = mPackageDexUsage.record(searchResult.mOwningPackageName,
                     dexPath, userId, isa, /*primaryOrSplit*/ false,
                     loadingPackage,
-                    PackageDexUsage.VARIABLE_CLASS_LOADER_CONTEXT);
+                    PackageDexUsage.VARIABLE_CLASS_LOADER_CONTEXT,
+                    /*overwriteCLC*/ false);
             update |= newUpdate;
         }
         if (update) {
@@ -809,7 +819,7 @@ public class DexManager {
         // Note: We don't have any way to detect which code paths are actually
         // owned by system server. We can only assume that such paths are on
         // system partitions.
-        if (PLATFORM_PACKAGE_NAME.equals(loadingAppInfo.packageName)) {
+        if (isPlatformPackage(loadingAppInfo.packageName)) {
             if (isSystemServerDexPathSupportedForOdex(dexPath)) {
                 // We record system server dex files as secondary dex files.
                 // The reason is that we only record the class loader context for secondary dex
@@ -842,6 +852,11 @@ public class DexManager {
         return new DexSearchResult(null, DEX_SEARCH_NOT_FOUND);
     }
 
+    /** Returns true if this is the platform package .*/
+    private static boolean isPlatformPackage(String packageName) {
+        return PLATFORM_PACKAGE_NAME.equals(packageName);
+    }
+
     private static <K,V> V putIfAbsent(Map<K,V> map, K key, V newValue) {
         V existingValue = map.putIfAbsent(key, newValue);
         return existingValue == null ? newValue : existingValue;

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

@@ -111,17 +111,18 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
      * @param dexPath the path of the dex files being loaded
      * @param ownerUserId the user id which runs the code loading the dex files
      * @param loaderIsa the ISA of the app loading the dex files
-     * @param isUsedByOtherApps whether or not this dex file was not loaded by its owning package
      * @param primaryOrSplit whether or not the dex file is a primary/split dex. True indicates
      *        the file is either primary or a split. False indicates the file is secondary dex.
      * @param loadingPackageName the package performing the load. Recorded only if it is different
      *        than {@param owningPackageName}.
+     * @param overwriteCLC if true, the class loader context will be overwritten instead of being
+     *        merged
      * @return true if the dex load constitutes new information, or false if this information
      *         has been seen before.
      */
     /* package */ boolean record(String owningPackageName, String dexPath, int ownerUserId,
             String loaderIsa, boolean primaryOrSplit,
-            String loadingPackageName, String classLoaderContext) {
+            String loadingPackageName, String classLoaderContext, boolean overwriteCLC) {
         if (!PackageManagerServiceUtils.checkISA(loaderIsa)) {
             throw new IllegalArgumentException("loaderIsa " + loaderIsa + " is unsupported");
         }
@@ -193,7 +194,7 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
                         }
                         // Merge the information into the existing data.
                         // Returns true if there was an update.
-                        return existingData.merge(newData) || updateLoadingPackages;
+                        return existingData.merge(newData, overwriteCLC) || updateLoadingPackages;
                     }
                 }
             }
@@ -809,14 +810,16 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
             mLoadingPackages = new HashSet<>(other.mLoadingPackages);
         }
 
-        private boolean merge(DexUseInfo dexUseInfo) {
+        private boolean merge(DexUseInfo dexUseInfo, boolean overwriteCLC) {
             boolean oldIsUsedByOtherApps = mIsUsedByOtherApps;
             mIsUsedByOtherApps = mIsUsedByOtherApps || dexUseInfo.mIsUsedByOtherApps;
             boolean updateIsas = mLoaderIsas.addAll(dexUseInfo.mLoaderIsas);
             boolean updateLoadingPackages = mLoadingPackages.addAll(dexUseInfo.mLoadingPackages);
 
             String oldClassLoaderContext = mClassLoaderContext;
-            if (isUnsupportedContext(mClassLoaderContext)) {
+            if (overwriteCLC) {
+                mClassLoaderContext = dexUseInfo.mClassLoaderContext;
+            } else if (isUnsupportedContext(mClassLoaderContext)) {
                 mClassLoaderContext = dexUseInfo.mClassLoaderContext;
             } else if (!Objects.equals(mClassLoaderContext, dexUseInfo.mClassLoaderContext)) {
                 // We detected a context change.

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

@@ -86,6 +86,7 @@ public class DexManagerTests {
     private TestData mBarUser0DelegateLastClassLoader;
 
     private TestData mSystemServerJar;
+    private TestData mSystemServerJarUpdatedContext;
     private TestData mSystemServerJarInvalid;
 
     private int mUser0;
@@ -113,6 +114,8 @@ public class DexManagerTests {
 
         mSystemServerJar = new TestData("android", isa, mUser0, PATH_CLASS_LOADER_NAME);
         mSystemServerJarInvalid = new TestData("android", isa, mUser0, PATH_CLASS_LOADER_NAME);
+        mSystemServerJarUpdatedContext = new TestData("android", isa, mUser0,
+                DELEGATE_LAST_CLASS_LOADER_NAME);
 
         mDexManager = new DexManager(/*Context*/ null, mPM, /*PackageDexOptimizer*/ null,
                 mInstaller, mInstallLock);
@@ -521,6 +524,24 @@ public class DexManagerTests {
                 expectedContexts);
     }
 
+    @Test
+    public void testSystemServerOverwritesContext() {
+        // Record bar secondaries with the default PathClassLoader.
+        List<String> secondaries = mSystemServerJar.getSecondaryDexPaths();
+
+        notifyDexLoad(mSystemServerJar, secondaries, mUser0);
+        PackageUseInfo pui = getPackageUseInfo(mSystemServerJar);
+        assertSecondaryUse(mSystemServerJar, pui, secondaries, /*isUsedByOtherApps*/false, mUser0);
+
+        // Record bar secondaries again with a different class loader. This will change the context.
+        notifyDexLoad(mSystemServerJarUpdatedContext, secondaries, mUser0);
+
+        pui = getPackageUseInfo(mSystemServerJar);
+        // We expect that all the contexts to be updated according to the last notify.
+        assertSecondaryUse(mSystemServerJarUpdatedContext, pui, secondaries,
+                /*isUsedByOtherApps*/false, mUser0);
+    }
+
     @Test
     public void testNotifyUnsupportedClassLoaderDoesNotChangeExisting() {
         List<String> secondaries = mBarUser0.getSecondaryDexPaths();

services/tests/servicestests/src/com/android/server/pm/dex/OWNERS

@@ -0,0 +1 @@
+include /services/core/java/com/android/server/pm/dex/OWNERS

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

@@ -451,7 +451,7 @@ public class PackageDexUsageTests {
                 "PCL[new_context.dex]");
         assertTrue(record(fooSecondary1User0NewContext));
 
-        // Not check that the context was switch to variable.
+        // Now check that the context was switch to variable.
         TestData expectedContext = mFooSecondary1User0.updateClassLoaderContext(
                 PackageDexUsage.VARIABLE_CLASS_LOADER_CONTEXT);
 
@@ -460,6 +460,22 @@ public class PackageDexUsageTests {
         assertPackageDexUsage(null, expectedContext);
     }
 
+    @Test
+    public void testRecordClassLoaderContextOverwritten() {
+        // Record a secondary dex file.
+        assertTrue(record(mFooSecondary1User0));
+        // Now update its context.
+        TestData fooSecondary1User0NewContext = mFooSecondary1User0.updateClassLoaderContext(
+                "PCL[new_context.dex]", true);
+        assertTrue(record(fooSecondary1User0NewContext));
+
+        // Now check that the context was overwritten.
+        TestData expectedContext = mFooSecondary1User0.updateClassLoaderContext(
+                "PCL[new_context.dex]", true);
+
+        assertPackageDexUsage(null, expectedContext);
+    }
+
     @Test
     public void testDexUsageClassLoaderContext() {
         final boolean isUsedByOtherApps = false;
@@ -642,8 +658,9 @@ public class PackageDexUsageTests {
 
     private boolean record(TestData testData) {
         return mPackageDexUsage.record(testData.mPackageName, testData.mDexFile,
-               testData.mOwnerUserId, testData.mLoaderIsa,
+                testData.mOwnerUserId, testData.mLoaderIsa,
-               testData.mPrimaryOrSplit, testData.mUsedBy, testData.mClassLoaderContext);
+                testData.mPrimaryOrSplit, testData.mUsedBy, testData.mClassLoaderContext,
+                testData.mOverwriteCLC);
     }
 
     private boolean record(PackageDexUsage packageDexUsage, TestData testData, Set<String> users) {
@@ -651,7 +668,8 @@ public class PackageDexUsageTests {
         for (String user : users) {
             result = result && packageDexUsage.record(testData.mPackageName, testData.mDexFile,
                     testData.mOwnerUserId, testData.mLoaderIsa,
-                    testData.mPrimaryOrSplit, user, testData.mClassLoaderContext);
+                    testData.mPrimaryOrSplit, user, testData.mClassLoaderContext,
+                    testData.mOverwriteCLC);
         }
         return result;
     }
@@ -682,15 +700,16 @@ public class PackageDexUsageTests {
         private final boolean mPrimaryOrSplit;
         private final String mUsedBy;
         private final String mClassLoaderContext;
+        private final boolean mOverwriteCLC;
 
         private TestData(String packageName, String dexFile, int ownerUserId,
                 String loaderIsa, boolean primaryOrSplit, String usedBy) {
             this(packageName, dexFile, ownerUserId, loaderIsa, primaryOrSplit,
-                    usedBy, "PCL[" + dexFile + "]");
+                    usedBy, "PCL[" + dexFile + "]", false);
         }
         private TestData(String packageName, String dexFile, int ownerUserId,
                 String loaderIsa, boolean primaryOrSplit, String usedBy,
-                String classLoaderContext) {
+                String classLoaderContext, boolean overwriteCLC) {
             mPackageName = packageName;
             mDexFile = dexFile;
             mOwnerUserId = ownerUserId;
@@ -698,16 +717,21 @@ public class PackageDexUsageTests {
             mPrimaryOrSplit = primaryOrSplit;
             mUsedBy = usedBy;
             mClassLoaderContext = classLoaderContext;
+            mOverwriteCLC = overwriteCLC;
         }
 
         private TestData updateClassLoaderContext(String newContext) {
+            return updateClassLoaderContext(newContext, mOverwriteCLC);
+        }
+
+        private TestData updateClassLoaderContext(String newContext, boolean overwriteCLC) {
             return new TestData(mPackageName, mDexFile, mOwnerUserId, mLoaderIsa,
-                    mPrimaryOrSplit, mUsedBy, newContext);
+                    mPrimaryOrSplit, mUsedBy, newContext, overwriteCLC);
         }
 
         private TestData updateUsedBy(String newUsedBy) {
             return new TestData(mPackageName, mDexFile, mOwnerUserId, mLoaderIsa,
-                mPrimaryOrSplit, newUsedBy, mClassLoaderContext);
+                mPrimaryOrSplit, newUsedBy, mClassLoaderContext, mOverwriteCLC);
         }
 
         private boolean isUsedByOtherApps() {